diff --git a/README.md b/README.md index a1ff8bba3..2548a4134 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` | + +> **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..40e2ade20 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" + << " 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; diff --git a/src/c2pool/main_btc.cpp b/src/c2pool/main_btc.cpp index e07e5102b..4c08385f3 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" + " 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; diff --git a/src/c2pool/main_dash.cpp b/src/c2pool/main_dash.cpp index 93c49ee07..b9ba0c6aa 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" + << " 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) diff --git a/src/c2pool/main_dgb.cpp b/src/c2pool/main_dgb.cpp index 11f8c97cd..e03751ce5 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" + << " 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) { diff --git a/src/c2pool/main_ltc.cpp b/src/c2pool/main_ltc.cpp index af5e51deb..e75d6627e 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 << " (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 @@ void print_help() { } 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 @@ 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/c2pool/merged/merged_mining.cpp b/src/c2pool/merged/merged_mining.cpp index 768b5c0d9..5558c9f43 100644 --- a/src/c2pool/merged/merged_mining.cpp +++ b/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(); } diff --git a/src/core/filesystem.hpp b/src/core/filesystem.hpp index 15f925ad8..f024efb2d 100644 --- a/src/core/filesystem.hpp +++ b/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. 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/src/core/test/filesystem_test.cpp b/src/core/test/filesystem_test.cpp new file mode 100644 index 000000000..ae6429da3 --- /dev/null +++ b/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 + +#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 +} + +// 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 diff --git a/src/core/web_server.cpp b/src/core/web_server.cpp index b74dab200..e7bcab965 100644 --- a/src/core/web_server.cpp +++ b/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);