Add --data-dir flag to isolate per-instance state (#722)

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` |
+
+> **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
 

src/c2pool/main_bch.cpp

@@ -39,6 +39,7 @@
 #include <btclibs/util/strencodings.h>   // ParseHexBytes (sharechain prefix)
 
 #include <core/core_util.hpp>
+#include <core/filesystem.hpp>   // core::filesystem::set_data_dir (--data-dir, #722)
 
 #include <cstdlib>
 #include <fstream>
@@ -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"
+        << "                   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,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) {
+            // 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;

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"
+        "                  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"
@@ -139,6 +142,16 @@ int main(int argc, char* argv[])
             print_usage();
             return 0;
         }
+        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")
         {
             testnet = true;

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"
+        << "                   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,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) {
+            // 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)

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"
+        << "                   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,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) {
+            // 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) {

src/c2pool/main_ltc.cpp

@@ -368,6 +368,8 @@
     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 << "                            (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";
@@ -523,6 +525,20 @@
 }
 
 int main(int argc, char* argv[]) {
+    // 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] == '-') {
+                std::cerr << "error: --data-dir requires a PATH argument\n";
+                return 1;
+            }
+            core::filesystem::set_data_dir(argv[i + 1]);
+        }
+    }
+
     // Install crash handlers
     std::set_terminate(c2pool_terminate_handler);
     std::signal(SIGINT, signal_handler);
@@ -783,6 +799,13 @@
             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");

src/c2pool/merged/merged_mining.cpp

@@ -1023,6 +1023,12 @@ void MergedMiningManager::try_submit_merged_blocks(
                          << "] Merged block submitted (" << block_hex.size()/2 << " bytes"
                          << ", snapshot hash=" << committed_block_hash.GetHex().substr(0, 16) << ")";
                 {
+                    // 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(); }

src/core/filesystem.hpp

@@ -10,8 +10,38 @@ 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). 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. 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.

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)
 

src/core/test/filesystem_test.cpp

@@ -0,0 +1,88 @@
+// 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,
+// 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()   (operator-supplied CLI flag)
+//   2. historical platform default          (byte-for-byte unchanged)
+//
+// 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 <gtest/gtest.h>
+
+#include <cstdlib>
+#include <filesystem>
+#include <string>
+
+#include <core/filesystem.hpp>
+
+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
+}
+
+// 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(""); }
+    void TearDown() override { core::filesystem::set_data_dir(""); }
+};
+
+// Tier 2: no --data-dir -> exact historical default (ship-critical parity).
+TEST_F(DataDirTest, DefaultUnchangedWhenUnset)
+{
+    EXPECT_EQ(core::filesystem::config_path(), historical_default());
+}
+
+// Tier 1: --data-dir (set_data_dir) overrides the platform default.
+TEST_F(DataDirTest, CliOverrideTakesEffect)
+{
+    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 to the default 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

src/core/web_server.cpp

@@ -1913,7 +1913,12 @@ 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. 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::string path = "/tmp/c2pool_block_" + std::to_string(std::time(nullptr)) + ".hex";
             std::ofstream f(path);