From bd512e4ec7b5df285517c4f12fa70b0f2cb44d17 Mon Sep 17 00:00:00 2001 From: frstrtr Date: Tue, 21 Jul 2026 05:20:52 +0400 Subject: [PATCH 1/6] Add --data-dir flag to isolate per-instance state (#722) Co-located c2pool instances opened the same ~/.c2pool LevelDB and contended its LOCK file, because every state path derived from the single hard-coded core::filesystem::config_path(). Whichever instance grabbed the lock first won; the other failed opaquely. This blocked safe multi-instance smoke/soak on one host and is reboot-fragile. Add a --data-dir PATH flag (mirroring bitcoind/litecoind -datadir) that re-roots ALL per-instance on-disk state: the sharechain LevelDB, addr/peer store, whitelist, v36 ratchet, found-blocks db, logs, and crash dumps. The fix lands at the one chokepoint config_path(), so every LevelDB open, addr store, and log sink follows automatically -- no call site changes. Resolution order in config_path(): 1. --data-dir PATH (set_data_dir(), highest priority) 2. C2POOL_DATA_DIR environment variable (config-less isolation) 3. historical platform default (unchanged) When neither is set, config_path() returns the exact prior expression, so single-instance behavior is byte-for-byte unchanged; the override is opt-in. Storage-path only -- zero share/target/vardiff/payout/coinbase logic touched (consensus-neutral; diff-grep empty). Flag wired into all five coin binaries (ltc, btc, dash, dgb, bch) plus README defaults/override table. The coin-daemon conf lookups reading getenv("HOME") directly (.dashcore, .digibyte, bch-regtest) are intentionally left alone -- they point at the external daemon's own datadir, not c2pool state. --- README.md | 7 +++++++ src/c2pool/main_bch.cpp | 10 +++++++++- src/c2pool/main_btc.cpp | 9 +++++++++ src/c2pool/main_dash.cpp | 7 +++++++ src/c2pool/main_dgb.cpp | 9 ++++++++- src/c2pool/main_ltc.cpp | 9 +++++++++ src/core/filesystem.hpp | 32 ++++++++++++++++++++++++++++++++ 7 files changed, 81 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index a1ff8bba3..86acf6b0e 100644 --- a/README.md +++ b/README.md @@ -247,6 +247,13 @@ Running `c2pool` with no arguments is equivalent to: | Stratum port | 9327 | `-w PORT` | | P2P port | 9326 | `--p2pool-port PORT` | | Web port | 8080 | `--web-port PORT` | +| Data directory | `~/.c2pool` (`%APPDATA%\c2pool` on Windows) | `--data-dir PATH` (or `C2POOL_DATA_DIR` env) | + +> **Running two instances on one host?** Give each its own `--data-dir`. +> All per-instance on-disk state — the sharechain LevelDB, address store, +> whitelist, logs, ratchet, and found-blocks db — is rooted there, so +> co-located instances never contend the same LevelDB `LOCK`. Leaving it +> unset keeps the historical `~/.c2pool` path, byte-for-byte unchanged. ### Testnet overrides diff --git a/src/c2pool/main_bch.cpp b/src/c2pool/main_bch.cpp index f5fc3d9f9..37012c5d1 100644 --- a/src/c2pool/main_bch.cpp +++ b/src/c2pool/main_bch.cpp @@ -39,6 +39,7 @@ #include // ParseHexBytes (sharechain prefix) #include +#include // core::filesystem::set_data_dir (--data-dir, #722) #include #include @@ -75,7 +76,9 @@ void print_banner(const char* argv0) << " " << argv0 << " --with-peer-verify [--testnet] [--peer HOST:PORT] [--max-seconds N]\n" << " " << argv0 << " --leg-c-capture [--rpc-conf PATH]\n" << " " << argv0 << " --leg-c-capture-p2p [--rpc-conf PATH] [--p2p-port N]\n" - << " " << argv0 << " --pool [--testnet|--testnet4|--regtest] [--stratum [HOST:]PORT] [--peer HOST:PORT] [--anchor N] [--rpc-conf PATH]\n\n" + << " " << argv0 << " --pool [--testnet|--testnet4|--regtest] [--stratum [HOST:]PORT] [--peer HOST:PORT] [--anchor N] [--rpc-conf PATH]\n" + << " --data-dir PATH root all per-instance state here (default ~/.c2pool;\n" + << " also C2POOL_DATA_DIR env); isolates co-located instances\n\n" << "Status: M5 pool/sharechain + embedded-daemon assembly live.\n" << " The embedded daemon (coin/embedded_daemon.hpp) is the primary\n" << " work source; external BCHN-RPC stays as the fallback.\n" @@ -711,6 +714,11 @@ int main(int argc, char** argv) return 0; } if (std::strcmp(argv[i], "--help") == 0) want_help = true; + if (std::strcmp(argv[i], "--data-dir") == 0 && i + 1 < argc) + // Root all per-instance state (LevelDB sharechain, addr store, + // logs, ...) under PATH so co-located instances don't contend the + // LevelDB LOCK. Default keeps ~/.c2pool. See #722. + core::filesystem::set_data_dir(argv[++i]); if (std::strcmp(argv[i], "--ibd") == 0) want_ibd = true; if (std::strcmp(argv[i], "--with-peer-verify") == 0) want_with_peer_verify = true; if (std::strcmp(argv[i], "--pool") == 0) want_pool = true; diff --git a/src/c2pool/main_btc.cpp b/src/c2pool/main_btc.cpp index e07e5102b..bd5451ca7 100644 --- a/src/c2pool/main_btc.cpp +++ b/src/c2pool/main_btc.cpp @@ -78,6 +78,9 @@ static void print_usage() " --testnet4 BTC testnet4 chain (genesis 00000000da84f2ba...)\n" " --regtest BTC regtest chain (genesis 0f9188f13cb7b2c7...)\n" " default: mainnet\n" + " --data-dir PATH root all per-instance state here (default ~/.c2pool;\n" + " also C2POOL_DATA_DIR env). Isolates co-located\n" + " instances so they don't contend the LevelDB LOCK.\n" " --bitcoind H:P bitcoind P2P endpoint host:port\n" " e.g. 127.0.0.1:8333 (mainnet)\n" " 127.0.0.1:18333 (testnet3)\n" @@ -139,6 +142,12 @@ int main(int argc, char* argv[]) print_usage(); return 0; } + else if (arg == "--data-dir" && i + 1 < argc) + { + // Isolate per-instance on-disk state (LevelDB sharechain, addr + // store, logs, ...) under PATH. Default keeps ~/.c2pool. See #722. + core::filesystem::set_data_dir(argv[++i]); + } else if (arg == "--testnet") { testnet = true; diff --git a/src/c2pool/main_dash.cpp b/src/c2pool/main_dash.cpp index 93c49ee07..f1036690a 100644 --- a/src/c2pool/main_dash.cpp +++ b/src/c2pool/main_dash.cpp @@ -191,6 +191,8 @@ void print_banner(const char* argv0) std::cout << "c2pool-dash " << C2POOL_VERSION << " — DASH (X11, older-than-v35 -> V36)\n\n" << "Usage: " << argv0 << " [--version] [--help] [--selftest]\n" + << " --data-dir PATH root all per-instance state here (default ~/.c2pool;\n" + << " also C2POOL_DATA_DIR env); isolates co-located instances\n" << " " << argv0 << " --run [--coin-rpc H:P] [--coin-rpc-auth PATH]\n" << " [--testnet] [--submit-block HEX | --submit-block-file PATH]\n" << " [--listen [HOST:]PORT] [--addnode HOST:PORT]... [--connect HOST:PORT]...\n" @@ -2199,6 +2201,11 @@ int main(int argc, char** argv) return 0; } else if (std::strcmp(argv[i], "--help") == 0) want_help = true; + else if (std::strcmp(argv[i], "--data-dir") == 0 && i + 1 < argc) + // Root all per-instance state (LevelDB sharechain, mn_state_db, + // addr store, logs, ...) under PATH so co-located instances don't + // contend the LevelDB LOCK. Default keeps ~/.c2pool. See #722. + core::filesystem::set_data_dir(argv[++i]); else if (std::strcmp(argv[i], "--run") == 0) want_run = true; else if (std::strcmp(argv[i], "--mine-block") == 0) want_mine = true; else if (std::strcmp(argv[i], "--payout-pubkey-hash") == 0 && i + 1 < argc) diff --git a/src/c2pool/main_dgb.cpp b/src/c2pool/main_dgb.cpp index 11f8c97cd..9df73d549 100644 --- a/src/c2pool/main_dgb.cpp +++ b/src/c2pool/main_dgb.cpp @@ -103,7 +103,9 @@ void print_banner(const char* argv0, const core::CoinParams& p) << " [--coin-daemon H:P] [--coin-magic HEX] [--regtest]\n" << " [--regtest-force-won-share] [--no-p2p-relay]\n" << " [--redistribute SPEC] [--sharechain-port P]\n" - << " [--dev-relax-algo-softforks]\n\n" + << " [--data-dir PATH] [--dev-relax-algo-softforks]\n\n" + << " --data-dir PATH root all per-instance state here (default ~/.c2pool;\n" + << " also C2POOL_DATA_DIR env); isolates co-located instances\n" << "Status: pool/sharechain pillars live (Phase B); run-loop up\n" << " (--run: io_context + sharechain peer + Stratum standup).\n" << " --stratum [HOST:]PORT binds a miner-facing TCP listener\n" @@ -1196,6 +1198,11 @@ int main(int argc, char** argv) return 0; } if (std::strcmp(argv[i], "--help") == 0) want_help = true; + if (std::strcmp(argv[i], "--data-dir") == 0 && i + 1 < argc) + // Root all per-instance state (LevelDB sharechain, addr store, + // logs, ...) under PATH so co-located instances don't contend the + // LevelDB LOCK. Default keeps ~/.c2pool. See #722. + core::filesystem::set_data_dir(argv[++i]); if (std::strcmp(argv[i], "--selftest") == 0) want_selftest = true; if (std::strcmp(argv[i], "--run") == 0) want_run = true; if (std::strcmp(argv[i], "--stratum") == 0 && i + 1 < argc) { diff --git a/src/c2pool/main_ltc.cpp b/src/c2pool/main_ltc.cpp index af5e51deb..2de8338d5 100644 --- a/src/c2pool/main_ltc.cpp +++ b/src/c2pool/main_ltc.cpp @@ -368,6 +368,8 @@ void print_help() { std::cout << "COMMAND LINE OPTIONS:\n"; std::cout << "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n"; std::cout << " --help, -h Show this help message and exit\n"; + std::cout << " --data-dir PATH Root all per-instance state here (default: ~/.c2pool)\n"; + std::cout << " (also settable via C2POOL_DATA_DIR env; isolates co-located instances)\n"; std::cout << " --testnet Use testnet instead of mainnet\n"; std::cout << " --integrated Full P2P pool with sharechain (DEFAULT)\n"; std::cout << " --solo Solo pool mode (no P2P sharechain, local payouts)\n"; @@ -783,6 +785,13 @@ int main(int argc, char* argv[]) { print_help(); return 0; } + else if (arg == "--data-dir" && i + 1 < argc) { + // Root ALL per-instance on-disk state (sharechain LevelDB, addr + // store, whitelist, logs, ratchet, found-blocks db, ...) under + // this path so co-located instances never contend the LevelDB + // LOCK. Default (unset) keeps ~/.c2pool — see issue #722. + core::filesystem::set_data_dir(argv[++i]); + } else if (arg == "--testnet") { settings->m_testnet = true; cli_explicit.insert("testnet"); diff --git a/src/core/filesystem.hpp b/src/core/filesystem.hpp index 15f925ad8..6585942a6 100644 --- a/src/core/filesystem.hpp +++ b/src/core/filesystem.hpp @@ -10,8 +10,40 @@ namespace core namespace filesystem { +// Process-wide data-dir override. Empty = unset. +// Populated once at startup from the `--data-dir PATH` CLI flag (via +// set_data_dir()). When set, it becomes the root of ALL per-instance +// on-disk state (sharechain LevelDB, addr store, whitelist, logs, ratchet, +// found-blocks db, etc.) so co-located c2pool instances can each own an +// isolated namespace and never contend the same LevelDB LOCK. See issue #722. +inline std::filesystem::path& data_dir_override() +{ + static std::filesystem::path s_override; + return s_override; +} + +// Set the per-instance data directory. Call once, before any code opens +// LevelDB or writes state (i.e. during CLI arg parsing). An empty path +// leaves the platform default in force. +inline void set_data_dir(const std::filesystem::path& p) +{ + data_dir_override() = p; +} + inline std::filesystem::path config_path() { + // 1. Explicit --data-dir override (highest priority). + const std::filesystem::path& ov = data_dir_override(); + if (!ov.empty()) + return ov; + + // 2. C2POOL_DATA_DIR environment variable — a config-less way to isolate + // state on multi-instance hosts (mirrors bitcoind honoring env datadir). + if (const char* env = std::getenv("C2POOL_DATA_DIR"); env && env[0] != '\0') + return std::filesystem::path(env); + + // 3. Historical default — byte-for-byte identical to prior releases so + // single-instance behavior is unchanged. // getenv may return nullptr if the var is unset; std::filesystem::path(nullptr) // is UB. Fall back to the current directory so an unset HOME/APPDATA degrades // gracefully instead of crashing. From 9e904299d596884787075309cb918eda027c6dd5 Mon Sep 17 00:00:00 2001 From: frstrtr Date: Tue, 21 Jul 2026 06:25:05 +0400 Subject: [PATCH 2/6] =?UTF-8?q?dash(#722):=20fold=20review=20nits=20?= =?UTF-8?q?=E2=80=94=20re-root=20/tmp=20residue=20(doge=5Fblock/crash/bloc?= =?UTF-8?q?k=20hex)=20under=20data-dir,=20construct=20Settings=20after=20a?= =?UTF-8?q?rg-parse,=20guard=20--data-dir=20missing=20value?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/c2pool/main_bch.cpp | 7 ++++++- src/c2pool/main_btc.cpp | 6 +++++- src/c2pool/main_dash.cpp | 7 ++++++- src/c2pool/main_dgb.cpp | 7 ++++++- src/c2pool/main_ltc.cpp | 29 ++++++++++++++++++++++++++++- src/c2pool/merged/merged_mining.cpp | 10 +++++++++- src/core/web_server.cpp | 10 ++++++++-- 7 files changed, 68 insertions(+), 8 deletions(-) diff --git a/src/c2pool/main_bch.cpp b/src/c2pool/main_bch.cpp index 37012c5d1..634b6e58e 100644 --- a/src/c2pool/main_bch.cpp +++ b/src/c2pool/main_bch.cpp @@ -714,11 +714,16 @@ int main(int argc, char** argv) return 0; } if (std::strcmp(argv[i], "--help") == 0) want_help = true; - if (std::strcmp(argv[i], "--data-dir") == 0 && i + 1 < argc) + if (std::strcmp(argv[i], "--data-dir") == 0) { // Root all per-instance state (LevelDB sharechain, addr store, // logs, ...) under PATH so co-located instances don't contend the // LevelDB LOCK. Default keeps ~/.c2pool. See #722. + if (i + 1 >= argc || argv[i + 1][0] == '\0' || argv[i + 1][0] == '-') { + std::cerr << "error: --data-dir requires a PATH argument\n"; + return 1; + } core::filesystem::set_data_dir(argv[++i]); + } if (std::strcmp(argv[i], "--ibd") == 0) want_ibd = true; if (std::strcmp(argv[i], "--with-peer-verify") == 0) want_with_peer_verify = true; if (std::strcmp(argv[i], "--pool") == 0) want_pool = true; diff --git a/src/c2pool/main_btc.cpp b/src/c2pool/main_btc.cpp index bd5451ca7..7abc70b47 100644 --- a/src/c2pool/main_btc.cpp +++ b/src/c2pool/main_btc.cpp @@ -142,10 +142,14 @@ int main(int argc, char* argv[]) print_usage(); return 0; } - else if (arg == "--data-dir" && i + 1 < argc) + else if (arg == "--data-dir") { // Isolate per-instance on-disk state (LevelDB sharechain, addr // store, logs, ...) under PATH. Default keeps ~/.c2pool. See #722. + if (i + 1 >= argc || argv[i + 1][0] == '\0' || argv[i + 1][0] == '-') { + std::cerr << "error: --data-dir requires a PATH argument\n"; + return 1; + } core::filesystem::set_data_dir(argv[++i]); } else if (arg == "--testnet") diff --git a/src/c2pool/main_dash.cpp b/src/c2pool/main_dash.cpp index f1036690a..73e78d851 100644 --- a/src/c2pool/main_dash.cpp +++ b/src/c2pool/main_dash.cpp @@ -2201,11 +2201,16 @@ int main(int argc, char** argv) return 0; } else if (std::strcmp(argv[i], "--help") == 0) want_help = true; - else if (std::strcmp(argv[i], "--data-dir") == 0 && i + 1 < argc) + else if (std::strcmp(argv[i], "--data-dir") == 0) { // Root all per-instance state (LevelDB sharechain, mn_state_db, // addr store, logs, ...) under PATH so co-located instances don't // contend the LevelDB LOCK. Default keeps ~/.c2pool. See #722. + if (i + 1 >= argc || argv[i + 1][0] == '\0' || argv[i + 1][0] == '-') { + std::cerr << "error: --data-dir requires a PATH argument\n"; + return 1; + } core::filesystem::set_data_dir(argv[++i]); + } else if (std::strcmp(argv[i], "--run") == 0) want_run = true; else if (std::strcmp(argv[i], "--mine-block") == 0) want_mine = true; else if (std::strcmp(argv[i], "--payout-pubkey-hash") == 0 && i + 1 < argc) diff --git a/src/c2pool/main_dgb.cpp b/src/c2pool/main_dgb.cpp index 9df73d549..35bbf5f89 100644 --- a/src/c2pool/main_dgb.cpp +++ b/src/c2pool/main_dgb.cpp @@ -1198,11 +1198,16 @@ int main(int argc, char** argv) return 0; } if (std::strcmp(argv[i], "--help") == 0) want_help = true; - if (std::strcmp(argv[i], "--data-dir") == 0 && i + 1 < argc) + if (std::strcmp(argv[i], "--data-dir") == 0) { // Root all per-instance state (LevelDB sharechain, addr store, // logs, ...) under PATH so co-located instances don't contend the // LevelDB LOCK. Default keeps ~/.c2pool. See #722. + if (i + 1 >= argc || argv[i + 1][0] == '\0' || argv[i + 1][0] == '-') { + std::cerr << "error: --data-dir requires a PATH argument\n"; + return 1; + } core::filesystem::set_data_dir(argv[++i]); + } if (std::strcmp(argv[i], "--selftest") == 0) want_selftest = true; if (std::strcmp(argv[i], "--run") == 0) want_run = true; if (std::strcmp(argv[i], "--stratum") == 0 && i + 1 < argc) { diff --git a/src/c2pool/main_ltc.cpp b/src/c2pool/main_ltc.cpp index 2de8338d5..2f3aa80d0 100644 --- a/src/c2pool/main_ltc.cpp +++ b/src/c2pool/main_ltc.cpp @@ -74,6 +74,12 @@ #include #include +// Precomputed at startup (after the --data-dir pre-scan in main) so the POSIX +// signal-handler path build is allocation-free. Empty until set; the handler +// falls back to /tmp when a crash predates initialization. Rooted under +// config_path() so co-located instances keep separate crash logs (#722). +static std::string g_crash_log_path; + // --- Platform-specific crash handler --- #ifdef _WIN32 #include @@ -189,7 +195,11 @@ static void segfault_handler(int sig) { #include static void write_crash_log(const char* reason) { - int fd = open("/tmp/c2pool_crash.log", O_WRONLY | O_CREAT | O_APPEND, 0640); + // Precomputed path (config_path()/crash.log) honors --data-dir; fall back + // to the historical /tmp location if a crash predates its initialization. + const char* crash_path = g_crash_log_path.empty() + ? "/tmp/c2pool_crash.log" : g_crash_log_path.c_str(); + int fd = open(crash_path, O_WRONLY | O_CREAT | O_APPEND, 0640); if (fd < 0) return; FILE* f = fdopen(fd, "a"); if (!f) { close(fd); return; } @@ -525,6 +535,23 @@ void print_help() { } int main(int argc, char* argv[]) { + // Pre-scan for --data-dir BEFORE anything reads config_path() — the crash + // handlers, the Settings ctor (Fileconfig captures its path at + // construction), and the file-log sink all resolve through it. Redirecting + // the chokepoint here guarantees the override wins everywhere. See #722. + for (int i = 1; i < argc; ++i) { + if (std::string(argv[i]) == "--data-dir") { + if (i + 1 >= argc || argv[i + 1][0] == '\0' || argv[i + 1][0] == '-') { + std::cerr << "error: --data-dir requires a PATH argument\n"; + return 1; + } + core::filesystem::set_data_dir(argv[i + 1]); + } + } + // Precompute the (now data-dir-aware) POSIX crash-log path for the + // async-signal-safe handler above. + g_crash_log_path = (core::filesystem::config_path() / "crash.log").string(); + // Install crash handlers std::set_terminate(c2pool_terminate_handler); std::signal(SIGINT, signal_handler); diff --git a/src/c2pool/merged/merged_mining.cpp b/src/c2pool/merged/merged_mining.cpp index 768b5c0d9..72c853e0c 100644 --- a/src/c2pool/merged/merged_mining.cpp +++ b/src/c2pool/merged/merged_mining.cpp @@ -3,6 +3,7 @@ #include #include +#include // config_path() — data-dir-aware block-hex dump (#722) #include #include @@ -1023,7 +1024,14 @@ void MergedMiningManager::try_submit_merged_blocks( << "] Merged block submitted (" << block_hex.size()/2 << " bytes" << ", snapshot hash=" << committed_block_hash.GetHex().substr(0, 16) << ")"; { - std::string path = "/tmp/c2pool_doge_block_" + std::to_string(chain.current_work.height) + ".hex"; + // Root under config_path()/tmp so co-located instances at + // the same child height don't overwrite each other's dump + // (was a deterministic /tmp collision). Honors --data-dir. + std::error_code dir_ec; + auto tmp_dir = core::filesystem::config_path() / "tmp"; + std::filesystem::create_directories(tmp_dir, dir_ec); + auto path = tmp_dir / ("c2pool_doge_block_" + + std::to_string(chain.current_work.height) + ".hex"); std::ofstream f(path); if (f.is_open()) { f << block_hex; f.close(); } } diff --git a/src/core/web_server.cpp b/src/core/web_server.cpp index b74dab200..41ffeb235 100644 --- a/src/core/web_server.cpp +++ b/src/core/web_server.cpp @@ -1913,9 +1913,15 @@ nlohmann::json MiningInterface::submitblock(const std::string& hex_data, const s } } - // Save block hex to file for manual submitblock testing + // Save block hex to file for manual submitblock testing. + // Root under config_path()/tmp so co-located instances stay isolated + // and the dump honors --data-dir (#722). { - std::string path = "/tmp/c2pool_block_" + std::to_string(std::time(nullptr)) + ".hex"; + std::error_code dir_ec; + auto tmp_dir = core::filesystem::config_path() / "tmp"; + std::filesystem::create_directories(tmp_dir, dir_ec); + std::string path = (tmp_dir / ("c2pool_block_" + + std::to_string(std::time(nullptr)) + ".hex")).string(); std::ofstream f(path); if (f) { f << hex_data; From 15acd26bc79a7110fbf9dc33c96a4365cf2a7b2c Mon Sep 17 00:00:00 2001 From: frstrtr Date: Wed, 22 Jul 2026 01:20:57 +0400 Subject: [PATCH 3/6] dash(#722): add config_path() data-dir resolution test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pin the --data-dir chokepoint (core::filesystem::config_path) three-tier resolution so co-located instances stay isolated and the single-instance default never regresses: - DefaultUnchangedWhenUnset: config_path() == historical platform default (cross-checked against an independent recompute) when neither flag nor C2POOL_DATA_DIR is set — the ship-critical byte-parity invariant. - EnvOverrideTakesEffect / CliOverrideBeatsEnv: env beats default, flag beats env. - ClearOverrideFallsBack: no sticky process state. - TwoInstancesGetDistinctRoots: two distinct --data-dir values yield disjoint state roots (each its own sharechain_leveldb — no LOCK contention). Header-only gtest (config_path is fully inline; no c2pool runtime link); registered in test/CMakeLists.txt and discovered by ctest. --- test/CMakeLists.txt | 10 +++ test/test_data_dir_isolation.cpp | 109 +++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 test/test_data_dir_isolation.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 464aac6a4..154ecc988 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -8,6 +8,16 @@ if (BUILD_TESTING AND GTest_FOUND) # had masked this, surfacing it on test_dgb_subsidy (the first PRE_TEST call). include(GoogleTest) + # --data-dir isolation chokepoint (issue #722). core/filesystem.hpp + # config_path() is fully inline/header-only; no c2pool runtime link + # required. MUST also be in the build.yml --target allowlist (#143). + add_executable(test_data_dir_isolation test_data_dir_isolation.cpp) + target_link_libraries(test_data_dir_isolation PRIVATE + GTest::gtest_main GTest::gtest + ${Boost_LIBRARIES} + ) + gtest_discover_tests(test_data_dir_isolation DISCOVERY_MODE PRE_TEST) + # DGB block subsidy oracle-conformance (card #156). config_coin.hpp # subsidy() is header-only/static; no dgb runtime link required. add_executable(test_dgb_subsidy test_dgb_subsidy.cpp) diff --git a/test/test_data_dir_isolation.cpp b/test/test_data_dir_isolation.cpp new file mode 100644 index 000000000..9f2c180da --- /dev/null +++ b/test/test_data_dir_isolation.cpp @@ -0,0 +1,109 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +// Regression test for the --data-dir isolation chokepoint (issue #722). +// +// core::filesystem::config_path() is the single root every per-instance state +// path derives from (sharechain LevelDB, addr store, whitelist, ratchet, +// found-blocks db, logs, crash/block hex dumps). This test pins its three-tier +// resolution order so co-located instances can each own an isolated namespace: +// +// 1. --data-dir PATH -> set_data_dir() (highest priority) +// 2. C2POOL_DATA_DIR -> environment (config-less isolation) +// 3. historical platform default (byte-for-byte unchanged) +// +// The default-parity case is the ship-critical invariant: with neither the +// flag nor the env var set, config_path() MUST return the exact prior +// expression so single-instance deployments are unaffected. + +#include + +#include +#include +#include + +#include + +namespace fs = std::filesystem; + +namespace { + +// Recompute the historical default independently of config_path() so the +// parity assertion is a genuine cross-check, not a tautology. +fs::path historical_default() +{ +#ifdef _WIN32 + const char* base = std::getenv("APPDATA"); + return fs::path(base ? base : ".") / "c2pool"; +#else + const char* base = std::getenv("HOME"); + return fs::path(base ? base : ".") / ".c2pool"; +#endif +} + +const char* kEnvVar = "C2POOL_DATA_DIR"; + +// Fixture that snapshots and restores the override + env var so tests do not +// leak process-wide state into one another (config_path() reads both). +class DataDirTest : public ::testing::Test { +protected: + void SetUp() override { + core::filesystem::set_data_dir(""); // clear CLI override + if (const char* prev = std::getenv(kEnvVar)) + m_saved_env = std::string(prev); + unsetenv(kEnvVar); + } + void TearDown() override { + core::filesystem::set_data_dir(""); + if (m_saved_env.has_value()) + setenv(kEnvVar, m_saved_env->c_str(), 1); + else + unsetenv(kEnvVar); + } + std::optional m_saved_env; +}; + +// Tier 3: neither flag nor env -> exact historical default (ship-critical). +TEST_F(DataDirTest, DefaultUnchangedWhenUnset) +{ + EXPECT_EQ(core::filesystem::config_path(), historical_default()); +} + +// Tier 2: env var wins over the platform default. +TEST_F(DataDirTest, EnvOverrideTakesEffect) +{ + setenv(kEnvVar, "/tmp/c2pool_env_instX", 1); + EXPECT_EQ(core::filesystem::config_path(), fs::path("/tmp/c2pool_env_instX")); +} + +// Tier 1: --data-dir (set_data_dir) wins over BOTH env and default. +TEST_F(DataDirTest, CliOverrideBeatsEnv) +{ + setenv(kEnvVar, "/tmp/c2pool_env_instX", 1); + core::filesystem::set_data_dir("/tmp/c2pool_cli_instY"); + EXPECT_EQ(core::filesystem::config_path(), fs::path("/tmp/c2pool_cli_instY")); +} + +// Clearing the override falls back through the chain again (no sticky state). +TEST_F(DataDirTest, ClearOverrideFallsBack) +{ + core::filesystem::set_data_dir("/tmp/c2pool_cli_instY"); + ASSERT_EQ(core::filesystem::config_path(), fs::path("/tmp/c2pool_cli_instY")); + core::filesystem::set_data_dir(""); + EXPECT_EQ(core::filesystem::config_path(), historical_default()); +} + +// The isolation guarantee itself: two distinct --data-dir values yield two +// distinct, non-overlapping state roots (each gets its own LevelDB namespace, +// so neither contends the other's LOCK). +TEST_F(DataDirTest, TwoInstancesGetDistinctRoots) +{ + core::filesystem::set_data_dir("/tmp/c2pool_instA"); + const fs::path a = core::filesystem::config_path(); + core::filesystem::set_data_dir("/tmp/c2pool_instB"); + const fs::path b = core::filesystem::config_path(); + + EXPECT_NE(a, b); + // The paths derived downstream (e.g. the sharechain LevelDB) stay disjoint. + EXPECT_NE(a / "sharechain_leveldb", b / "sharechain_leveldb"); +} + +} // namespace From 5fd53cb3ca07b9471deea854312478c5908cb0ba Mon Sep 17 00:00:00 2001 From: frstrtr Date: Wed, 22 Jul 2026 02:27:46 +0400 Subject: [PATCH 4/6] dash(#722): keep transient /tmp dumps off the --data-dir chokepoint (CodeQL path-injection) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier review-nit fold re-rooted three transient debug artifacts — the POSIX crash log (main_ltc), the merged doge-block hex dump (merged_mining), and the manual-submit block hex dump (web_server) — under config_path()/tmp so they honored --data-dir. But config_path() derives from an environment variable / CLI arg, so routing it into these file sinks introduced three new CodeQL cpp/path-injection (high) alerts on paths that were previously fixed string constants. These three are throwaway manual-test/debug artifacts, not the per-instance state that co-located isolation is actually about. Revert them to their historical fixed /tmp paths; the state that matters for the two-node soak — sharechain LevelDB, addr/peer store, whitelist, ratchet, found-blocks db, and the file-log sink — still re-roots through config_path() and remains fully isolated by --data-dir. Removes the precomputed g_crash_log_path and the now-unused core/filesystem include in merged_mining. Net: zero new path-injection sinks vs master; the --data-dir feature and its test are unchanged. --- src/c2pool/main_ltc.cpp | 23 +++++------------------ src/c2pool/merged/merged_mining.cpp | 16 +++++++--------- src/core/web_server.cpp | 15 +++++++-------- 3 files changed, 19 insertions(+), 35 deletions(-) diff --git a/src/c2pool/main_ltc.cpp b/src/c2pool/main_ltc.cpp index 2f3aa80d0..08996a8b8 100644 --- a/src/c2pool/main_ltc.cpp +++ b/src/c2pool/main_ltc.cpp @@ -74,12 +74,6 @@ #include #include -// Precomputed at startup (after the --data-dir pre-scan in main) so the POSIX -// signal-handler path build is allocation-free. Empty until set; the handler -// falls back to /tmp when a crash predates initialization. Rooted under -// config_path() so co-located instances keep separate crash logs (#722). -static std::string g_crash_log_path; - // --- Platform-specific crash handler --- #ifdef _WIN32 #include @@ -195,11 +189,7 @@ static void segfault_handler(int sig) { #include static void write_crash_log(const char* reason) { - // Precomputed path (config_path()/crash.log) honors --data-dir; fall back - // to the historical /tmp location if a crash predates its initialization. - const char* crash_path = g_crash_log_path.empty() - ? "/tmp/c2pool_crash.log" : g_crash_log_path.c_str(); - int fd = open(crash_path, O_WRONLY | O_CREAT | O_APPEND, 0640); + int fd = open("/tmp/c2pool_crash.log", O_WRONLY | O_CREAT | O_APPEND, 0640); if (fd < 0) return; FILE* f = fdopen(fd, "a"); if (!f) { close(fd); return; } @@ -535,10 +525,10 @@ void print_help() { } int main(int argc, char* argv[]) { - // Pre-scan for --data-dir BEFORE anything reads config_path() — the crash - // handlers, the Settings ctor (Fileconfig captures its path at - // construction), and the file-log sink all resolve through it. Redirecting - // the chokepoint here guarantees the override wins everywhere. See #722. + // Pre-scan for --data-dir BEFORE anything reads config_path() — the + // Settings ctor (Fileconfig captures its path at construction) and the + // file-log sink both resolve through it. Redirecting the chokepoint here + // guarantees the override wins everywhere. See #722. for (int i = 1; i < argc; ++i) { if (std::string(argv[i]) == "--data-dir") { if (i + 1 >= argc || argv[i + 1][0] == '\0' || argv[i + 1][0] == '-') { @@ -548,9 +538,6 @@ int main(int argc, char* argv[]) { core::filesystem::set_data_dir(argv[i + 1]); } } - // Precompute the (now data-dir-aware) POSIX crash-log path for the - // async-signal-safe handler above. - g_crash_log_path = (core::filesystem::config_path() / "crash.log").string(); // Install crash handlers std::set_terminate(c2pool_terminate_handler); diff --git a/src/c2pool/merged/merged_mining.cpp b/src/c2pool/merged/merged_mining.cpp index 72c853e0c..5558c9f43 100644 --- a/src/c2pool/merged/merged_mining.cpp +++ b/src/c2pool/merged/merged_mining.cpp @@ -3,7 +3,6 @@ #include #include -#include // config_path() — data-dir-aware block-hex dump (#722) #include #include @@ -1024,14 +1023,13 @@ void MergedMiningManager::try_submit_merged_blocks( << "] Merged block submitted (" << block_hex.size()/2 << " bytes" << ", snapshot hash=" << committed_block_hash.GetHex().substr(0, 16) << ")"; { - // Root under config_path()/tmp so co-located instances at - // the same child height don't overwrite each other's dump - // (was a deterministic /tmp collision). Honors --data-dir. - std::error_code dir_ec; - auto tmp_dir = core::filesystem::config_path() / "tmp"; - std::filesystem::create_directories(tmp_dir, dir_ec); - auto path = tmp_dir / ("c2pool_doge_block_" - + std::to_string(chain.current_work.height) + ".hex"); + // Transient manual-submit artifact kept at a fixed /tmp + // path (NOT re-rooted under --data-dir): routing an + // env/CLI-derived path into a file sink trips CodeQL + // cpp/path-injection, and the state that matters for + // co-located isolation (LevelDB, addrs, logs) is already + // re-rooted via config_path(). See #722. + std::string path = "/tmp/c2pool_doge_block_" + std::to_string(chain.current_work.height) + ".hex"; std::ofstream f(path); if (f.is_open()) { f << block_hex; f.close(); } } diff --git a/src/core/web_server.cpp b/src/core/web_server.cpp index 41ffeb235..e7bcab965 100644 --- a/src/core/web_server.cpp +++ b/src/core/web_server.cpp @@ -1913,15 +1913,14 @@ nlohmann::json MiningInterface::submitblock(const std::string& hex_data, const s } } - // Save block hex to file for manual submitblock testing. - // Root under config_path()/tmp so co-located instances stay isolated - // and the dump honors --data-dir (#722). + // Save block hex to file for manual submitblock testing. Kept at a + // fixed /tmp path (NOT re-rooted under --data-dir): it is a transient + // manual-test artifact, and routing an env/CLI-derived path into a + // file sink trips CodeQL cpp/path-injection with no isolation benefit + // (per-instance state that matters — LevelDB, addrs, logs — is already + // isolated via config_path()). See #722. { - std::error_code dir_ec; - auto tmp_dir = core::filesystem::config_path() / "tmp"; - std::filesystem::create_directories(tmp_dir, dir_ec); - std::string path = (tmp_dir / ("c2pool_block_" - + std::to_string(std::time(nullptr)) + ".hex")).string(); + std::string path = "/tmp/c2pool_block_" + std::to_string(std::time(nullptr)) + ".hex"; std::ofstream f(path); if (f) { f << hex_data; From f22413dc75cc85ba5a35aea72961c7c283941450 Mon Sep 17 00:00:00 2001 From: frstrtr Date: Wed, 22 Jul 2026 03:44:40 +0400 Subject: [PATCH 5/6] dash(#722): fold data-dir test into core_test (CI runs it, no NOT_BUILT sentinel) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The standalone test_data_dir_isolation target was registered in test/CMakeLists.txt but not added to build.yml's --target allowlist, so CI configured it, never built it, and ctest surfaced a test_data_dir_isolation_NOT_BUILT sentinel that failed the Run-tests step. Move the config_path() resolution cases into the existing core_test executable (src/core/test/filesystem_test.cpp), which is already in the CI --target allowlist and uses build-time gtest_add_tests(AUTO) discovery — so the test actually builds and runs in CI with no workflow change and no NOT_BUILT sentinel. Coverage is identical: default parity, env override, CLI precedence, clear-to-fallback, two-distinct-roots. --- src/core/test/CMakeLists.txt | 2 +- .../core/test/filesystem_test.cpp | 2 ++ test/CMakeLists.txt | 10 ---------- 3 files changed, 3 insertions(+), 11 deletions(-) rename test/test_data_dir_isolation.cpp => src/core/test/filesystem_test.cpp (96%) diff --git a/src/core/test/CMakeLists.txt b/src/core/test/CMakeLists.txt index 2bce14e47..4463c8766 100644 --- a/src/core/test/CMakeLists.txt +++ b/src/core/test/CMakeLists.txt @@ -1,5 +1,5 @@ if (BUILD_TESTING AND (GTest_FOUND OR GTEST_FOUND)) - add_executable(core_test pack_test.cpp events_test.cpp chain_test.cpp packet_test.cpp block_broadcast_test.cpp broadcast_convergence_matrix_test.cpp core_merkle_branches_test.cpp version_gate_test.cpp) + add_executable(core_test pack_test.cpp events_test.cpp chain_test.cpp packet_test.cpp block_broadcast_test.cpp broadcast_convergence_matrix_test.cpp core_merkle_branches_test.cpp version_gate_test.cpp filesystem_test.cpp) target_link_libraries(core_test PRIVATE GTest::gtest_main core c2pool_merged_mining GTest::gtest) target_link_libraries(core_test PRIVATE c2pool_payout c2pool_hashrate ltc_coin) # OBJECT-lib SCC direct-naming (#22/#39) diff --git a/test/test_data_dir_isolation.cpp b/src/core/test/filesystem_test.cpp similarity index 96% rename from test/test_data_dir_isolation.cpp rename to src/core/test/filesystem_test.cpp index 9f2c180da..b3a6a6745 100644 --- a/test/test_data_dir_isolation.cpp +++ b/src/core/test/filesystem_test.cpp @@ -1,5 +1,7 @@ // SPDX-License-Identifier: AGPL-3.0-or-later // Regression test for the --data-dir isolation chokepoint (issue #722). +// Lives in the core_test executable (already built + run in CI) so it is +// exercised without a dedicated build.yml --target entry. // // core::filesystem::config_path() is the single root every per-instance state // path derives from (sharechain LevelDB, addr store, whitelist, ratchet, diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 154ecc988..464aac6a4 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -8,16 +8,6 @@ if (BUILD_TESTING AND GTest_FOUND) # had masked this, surfacing it on test_dgb_subsidy (the first PRE_TEST call). include(GoogleTest) - # --data-dir isolation chokepoint (issue #722). core/filesystem.hpp - # config_path() is fully inline/header-only; no c2pool runtime link - # required. MUST also be in the build.yml --target allowlist (#143). - add_executable(test_data_dir_isolation test_data_dir_isolation.cpp) - target_link_libraries(test_data_dir_isolation PRIVATE - GTest::gtest_main GTest::gtest - ${Boost_LIBRARIES} - ) - gtest_discover_tests(test_data_dir_isolation DISCOVERY_MODE PRE_TEST) - # DGB block subsidy oracle-conformance (card #156). config_coin.hpp # subsidy() is header-only/static; no dgb runtime link required. add_executable(test_dgb_subsidy test_dgb_subsidy.cpp) From 94ecdae58dd0705f9abc1fbb33c9aa8348e4ba38 Mon Sep 17 00:00:00 2001 From: frstrtr Date: Wed, 22 Jul 2026 05:10:32 +0400 Subject: [PATCH 6/6] =?UTF-8?q?dash(#722):=20drop=20C2POOL=5FDATA=5FDIR=20?= =?UTF-8?q?env=20override=20=E2=80=94=20--data-dir=20is=20CLI-only=20(Code?= =?UTF-8?q?QL)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit config_path() previously honored a C2POOL_DATA_DIR environment variable as a config-less isolation shortcut. Reading it added a new getenv taint source that CodeQL cpp/path-injection traced through config_path() into the pool whitelist ifstream (sharechain_node.hpp), re-fingerprinting that pre-existing master-baseline alert as new-in-PR (1 high). The env override was a convenience beyond the issue's actual ask (a --data-dir flag). Drop it: the sole override input is now the operator-supplied CLI flag (set_data_dir from argv), so config_path()'s only environment reads remain the historical HOME/APPDATA — identical taint surface to master, no new alerts. The --data-dir flag, its five-binary wiring, and the isolation guarantee are unchanged. Test drops the env-precedence cases; help text / README drop the C2POOL_DATA_DIR mention. --- README.md | 2 +- src/c2pool/main_bch.cpp | 4 +-- src/c2pool/main_btc.cpp | 6 ++-- src/c2pool/main_dash.cpp | 4 +-- src/c2pool/main_dgb.cpp | 4 +-- src/c2pool/main_ltc.cpp | 2 +- src/core/filesystem.hpp | 12 +++---- src/core/test/filesystem_test.cpp | 53 +++++++++---------------------- 8 files changed, 31 insertions(+), 56 deletions(-) diff --git a/README.md b/README.md index 86acf6b0e..2548a4134 100644 --- a/README.md +++ b/README.md @@ -247,7 +247,7 @@ Running `c2pool` with no arguments is equivalent to: | Stratum port | 9327 | `-w PORT` | | P2P port | 9326 | `--p2pool-port PORT` | | Web port | 8080 | `--web-port PORT` | -| Data directory | `~/.c2pool` (`%APPDATA%\c2pool` on Windows) | `--data-dir PATH` (or `C2POOL_DATA_DIR` env) | +| Data directory | `~/.c2pool` (`%APPDATA%\c2pool` on Windows) | `--data-dir PATH` | > **Running two instances on one host?** Give each its own `--data-dir`. > All per-instance on-disk state — the sharechain LevelDB, address store, diff --git a/src/c2pool/main_bch.cpp b/src/c2pool/main_bch.cpp index 634b6e58e..40e2ade20 100644 --- a/src/c2pool/main_bch.cpp +++ b/src/c2pool/main_bch.cpp @@ -77,8 +77,8 @@ void print_banner(const char* argv0) << " " << argv0 << " --leg-c-capture [--rpc-conf PATH]\n" << " " << argv0 << " --leg-c-capture-p2p [--rpc-conf PATH] [--p2p-port N]\n" << " " << argv0 << " --pool [--testnet|--testnet4|--regtest] [--stratum [HOST:]PORT] [--peer HOST:PORT] [--anchor N] [--rpc-conf PATH]\n" - << " --data-dir PATH root all per-instance state here (default ~/.c2pool;\n" - << " also C2POOL_DATA_DIR env); isolates co-located instances\n\n" + << " --data-dir PATH root all per-instance state here (default ~/.c2pool);\n" + << " isolates co-located instances\n\n" << "Status: M5 pool/sharechain + embedded-daemon assembly live.\n" << " The embedded daemon (coin/embedded_daemon.hpp) is the primary\n" << " work source; external BCHN-RPC stays as the fallback.\n" diff --git a/src/c2pool/main_btc.cpp b/src/c2pool/main_btc.cpp index 7abc70b47..4c08385f3 100644 --- a/src/c2pool/main_btc.cpp +++ b/src/c2pool/main_btc.cpp @@ -78,9 +78,9 @@ static void print_usage() " --testnet4 BTC testnet4 chain (genesis 00000000da84f2ba...)\n" " --regtest BTC regtest chain (genesis 0f9188f13cb7b2c7...)\n" " default: mainnet\n" - " --data-dir PATH root all per-instance state here (default ~/.c2pool;\n" - " also C2POOL_DATA_DIR env). Isolates co-located\n" - " instances so they don't contend the LevelDB LOCK.\n" + " --data-dir PATH root all per-instance state here (default ~/.c2pool).\n" + " Isolates co-located instances so they don't contend\n" + " the LevelDB LOCK.\n" " --bitcoind H:P bitcoind P2P endpoint host:port\n" " e.g. 127.0.0.1:8333 (mainnet)\n" " 127.0.0.1:18333 (testnet3)\n" diff --git a/src/c2pool/main_dash.cpp b/src/c2pool/main_dash.cpp index 73e78d851..b9ba0c6aa 100644 --- a/src/c2pool/main_dash.cpp +++ b/src/c2pool/main_dash.cpp @@ -191,8 +191,8 @@ void print_banner(const char* argv0) std::cout << "c2pool-dash " << C2POOL_VERSION << " — DASH (X11, older-than-v35 -> V36)\n\n" << "Usage: " << argv0 << " [--version] [--help] [--selftest]\n" - << " --data-dir PATH root all per-instance state here (default ~/.c2pool;\n" - << " also C2POOL_DATA_DIR env); isolates co-located instances\n" + << " --data-dir PATH root all per-instance state here (default ~/.c2pool);\n" + << " isolates co-located instances\n" << " " << argv0 << " --run [--coin-rpc H:P] [--coin-rpc-auth PATH]\n" << " [--testnet] [--submit-block HEX | --submit-block-file PATH]\n" << " [--listen [HOST:]PORT] [--addnode HOST:PORT]... [--connect HOST:PORT]...\n" diff --git a/src/c2pool/main_dgb.cpp b/src/c2pool/main_dgb.cpp index 35bbf5f89..e03751ce5 100644 --- a/src/c2pool/main_dgb.cpp +++ b/src/c2pool/main_dgb.cpp @@ -104,8 +104,8 @@ void print_banner(const char* argv0, const core::CoinParams& p) << " [--regtest-force-won-share] [--no-p2p-relay]\n" << " [--redistribute SPEC] [--sharechain-port P]\n" << " [--data-dir PATH] [--dev-relax-algo-softforks]\n\n" - << " --data-dir PATH root all per-instance state here (default ~/.c2pool;\n" - << " also C2POOL_DATA_DIR env); isolates co-located instances\n" + << " --data-dir PATH root all per-instance state here (default ~/.c2pool);\n" + << " isolates co-located instances\n" << "Status: pool/sharechain pillars live (Phase B); run-loop up\n" << " (--run: io_context + sharechain peer + Stratum standup).\n" << " --stratum [HOST:]PORT binds a miner-facing TCP listener\n" diff --git a/src/c2pool/main_ltc.cpp b/src/c2pool/main_ltc.cpp index 08996a8b8..e75d6627e 100644 --- a/src/c2pool/main_ltc.cpp +++ b/src/c2pool/main_ltc.cpp @@ -369,7 +369,7 @@ void print_help() { std::cout << "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n"; std::cout << " --help, -h Show this help message and exit\n"; std::cout << " --data-dir PATH Root all per-instance state here (default: ~/.c2pool)\n"; - std::cout << " (also settable via C2POOL_DATA_DIR env; isolates co-located instances)\n"; + std::cout << " (isolates co-located instances)\n"; std::cout << " --testnet Use testnet instead of mainnet\n"; std::cout << " --integrated Full P2P pool with sharechain (DEFAULT)\n"; std::cout << " --solo Solo pool mode (no P2P sharechain, local payouts)\n"; diff --git a/src/core/filesystem.hpp b/src/core/filesystem.hpp index 6585942a6..f024efb2d 100644 --- a/src/core/filesystem.hpp +++ b/src/core/filesystem.hpp @@ -32,17 +32,15 @@ inline void set_data_dir(const std::filesystem::path& p) inline std::filesystem::path config_path() { - // 1. Explicit --data-dir override (highest priority). + // 1. Explicit --data-dir override (highest priority). This is the ONLY + // override input: it is set solely from the operator-supplied CLI flag, + // never from an environment variable, so config_path()'s only env reads + // remain the historical HOME/APPDATA below (unchanged taint surface). const std::filesystem::path& ov = data_dir_override(); if (!ov.empty()) return ov; - // 2. C2POOL_DATA_DIR environment variable — a config-less way to isolate - // state on multi-instance hosts (mirrors bitcoind honoring env datadir). - if (const char* env = std::getenv("C2POOL_DATA_DIR"); env && env[0] != '\0') - return std::filesystem::path(env); - - // 3. Historical default — byte-for-byte identical to prior releases so + // 2. Historical default — byte-for-byte identical to prior releases so // single-instance behavior is unchanged. // getenv may return nullptr if the var is unset; std::filesystem::path(nullptr) // is UB. Fall back to the current directory so an unset HOME/APPDATA degrades diff --git a/src/core/test/filesystem_test.cpp b/src/core/test/filesystem_test.cpp index b3a6a6745..ae6429da3 100644 --- a/src/core/test/filesystem_test.cpp +++ b/src/core/test/filesystem_test.cpp @@ -5,16 +5,15 @@ // // core::filesystem::config_path() is the single root every per-instance state // path derives from (sharechain LevelDB, addr store, whitelist, ratchet, -// found-blocks db, logs, crash/block hex dumps). This test pins its three-tier -// resolution order so co-located instances can each own an isolated namespace: +// found-blocks db, logs). This test pins its resolution order so co-located +// instances can each own an isolated namespace: // -// 1. --data-dir PATH -> set_data_dir() (highest priority) -// 2. C2POOL_DATA_DIR -> environment (config-less isolation) -// 3. historical platform default (byte-for-byte unchanged) +// 1. --data-dir PATH -> set_data_dir() (operator-supplied CLI flag) +// 2. historical platform default (byte-for-byte unchanged) // -// The default-parity case is the ship-critical invariant: with neither the -// flag nor the env var set, config_path() MUST return the exact prior -// expression so single-instance deployments are unaffected. +// The default-parity case is the ship-critical invariant: with no --data-dir +// set, config_path() MUST return the exact prior expression so single-instance +// deployments are unaffected. #include @@ -41,50 +40,28 @@ fs::path historical_default() #endif } -const char* kEnvVar = "C2POOL_DATA_DIR"; - -// Fixture that snapshots and restores the override + env var so tests do not -// leak process-wide state into one another (config_path() reads both). +// Fixture that clears the override before and after each test so tests do not +// leak process-wide state into one another (config_path() reads it). class DataDirTest : public ::testing::Test { protected: - void SetUp() override { - core::filesystem::set_data_dir(""); // clear CLI override - if (const char* prev = std::getenv(kEnvVar)) - m_saved_env = std::string(prev); - unsetenv(kEnvVar); - } - void TearDown() override { - core::filesystem::set_data_dir(""); - if (m_saved_env.has_value()) - setenv(kEnvVar, m_saved_env->c_str(), 1); - else - unsetenv(kEnvVar); - } - std::optional m_saved_env; + void SetUp() override { core::filesystem::set_data_dir(""); } + void TearDown() override { core::filesystem::set_data_dir(""); } }; -// Tier 3: neither flag nor env -> exact historical default (ship-critical). +// Tier 2: no --data-dir -> exact historical default (ship-critical parity). TEST_F(DataDirTest, DefaultUnchangedWhenUnset) { EXPECT_EQ(core::filesystem::config_path(), historical_default()); } -// Tier 2: env var wins over the platform default. -TEST_F(DataDirTest, EnvOverrideTakesEffect) -{ - setenv(kEnvVar, "/tmp/c2pool_env_instX", 1); - EXPECT_EQ(core::filesystem::config_path(), fs::path("/tmp/c2pool_env_instX")); -} - -// Tier 1: --data-dir (set_data_dir) wins over BOTH env and default. -TEST_F(DataDirTest, CliOverrideBeatsEnv) +// Tier 1: --data-dir (set_data_dir) overrides the platform default. +TEST_F(DataDirTest, CliOverrideTakesEffect) { - setenv(kEnvVar, "/tmp/c2pool_env_instX", 1); core::filesystem::set_data_dir("/tmp/c2pool_cli_instY"); EXPECT_EQ(core::filesystem::config_path(), fs::path("/tmp/c2pool_cli_instY")); } -// Clearing the override falls back through the chain again (no sticky state). +// Clearing the override falls back to the default again (no sticky state). TEST_F(DataDirTest, ClearOverrideFallsBack) { core::filesystem::set_data_dir("/tmp/c2pool_cli_instY");