From b9d71177125a760526a5df7948c4e0a67a1716ac Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Date: Thu, 25 Jun 2026 09:04:34 +0200 Subject: [PATCH] fix: Fix paths for logos core embedded in basecamp (#45) * Remove circuits directory setup * Pipe new paths machinery * Update logos-blockchain tip --- README.md | 2 +- flake.lock | 8 +- flake.nix | 32 +------ metadata.json | 3 +- src/logos_blockchain_module.cpp | 127 ++++++++++++++++++-------- src/logos_blockchain_module.h | 10 +- tests/mocks/mock_logos_blockchain.cpp | 13 +++ tests/stubs/logos_blockchain.h | 2 + tests/test_blockchain.cpp | 126 +++++++++++++++++++++++-- 9 files changed, 236 insertions(+), 87 deletions(-) diff --git a/README.md b/README.md index 1cca043..9edbbd0 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Logos Blockchain Module -A Logos core module that wraps the [logos-blockchain](https://github.com/logos-blockchain/logos-blockchain) C bindings and ships the zk circuit binaries needed at runtime. +A Logos core module that wraps the [logos-blockchain](https://github.com/logos-blockchain/logos-blockchain) C bindings. ### Build and inspect diff --git a/flake.lock b/flake.lock index 0e099ad..5a140fe 100644 --- a/flake.lock +++ b/flake.lock @@ -24,16 +24,16 @@ "rust-rapidsnark": "rust-rapidsnark" }, "locked": { - "lastModified": 1782212034, - "narHash": "sha256-IcU+c3YokqSmv4huTHZsBUhsen8lLs3wGvtaMWfQths=", + "lastModified": 1782334675, + "narHash": "sha256-EYDWycdTnS6krTf91Ot1lyceEoa5jiniNZRF1+Ddpnw=", "owner": "logos-blockchain", "repo": "logos-blockchain", - "rev": "113faae6fd5f68aa70e3cece4fa54f8522c95eec", + "rev": "58d71393cb7c5e8d425f54b09f2f28e0d9905fdc", "type": "github" }, "original": { "owner": "logos-blockchain", - "ref": "113faae6fd5f68aa70e3cece4fa54f8522c95eec", + "ref": "58d71393cb7c5e8d425f54b09f2f28e0d9905fdc", "repo": "logos-blockchain", "type": "github" } diff --git a/flake.nix b/flake.nix index 5980c94..47e00f7 100644 --- a/flake.nix +++ b/flake.nix @@ -3,8 +3,8 @@ inputs = { logos-module-builder.url = "github:logos-co/logos-module-builder?ref=38ddf92c1f240f4e420d300a1fbabb1609d5db01"; - # https://github.com/logos-blockchain/logos-blockchain/commit/113faae6fd5f68aa70e3cece4fa54f8522c95eec - logos-blockchain.url = "github:logos-blockchain/logos-blockchain?ref=113faae6fd5f68aa70e3cece4fa54f8522c95eec"; + # https://github.com/logos-blockchain/logos-blockchain/commit/58d71393cb7c5e8d425f54b09f2f28e0d9905fdc + logos-blockchain.url = "github:logos-blockchain/logos-blockchain?ref=58d71393cb7c5e8d425f54b09f2f28e0d9905fdc"; }; outputs = inputs@{ logos-module-builder, ... }: @@ -22,35 +22,7 @@ mockCLibs = [ "logos_blockchain" ]; }; - preConfigure = { externalLibs }: - if externalLibs ? logos_blockchain then '' - if [ -d "${externalLibs.logos_blockchain}/circuits" ]; then - echo "Staging zk circuits from logos-blockchain..." - cp -r "${externalLibs.logos_blockchain}/circuits" ./circuits - chmod -R u+w ./circuits - else - echo "WARNING: no circuits/ found in logos-blockchain derivation" - fi - '' else '' - echo "Skipping zk circuits staging (logos_blockchain mocked for tests)" - ''; - - # Logos Core Edge-case - # The current version of Logos Core expects circuits' binaries under `lib/circuits/`. - # Until we address this in Logos Core, we use this hook to include to ensure the circuits' binaries - # are included in the binary bundle and avoid the circuits being mangled by Nix (which did that when - # copying them in a previous phase). postInstall = '' - if [ -d "$LOGOS_MODULE_SOURCE_DIR/circuits" ]; then - cp -r "$LOGOS_MODULE_SOURCE_DIR/circuits" "$out/lib/circuits" - chmod -R u+w "$out/lib/circuits" - - if [ -f "$out/lib/circuits/lib/libgmp.a" ]; then - echo "Removing loose static library libgmp.a from staged circuits..." - rm "$out/lib/circuits/lib/libgmp.a" - fi - fi - # Remove nix references to make the module portable. find "$out" -type f | while read -r binary; do if file "$binary" | grep -E -q "Mach-O|shared library|executable|archive"; then diff --git a/metadata.json b/metadata.json index c289112..db97108 100644 --- a/metadata.json +++ b/metadata.json @@ -20,8 +20,7 @@ "liblogos_blockchain.dll", "liblogos_blockchain_module_plugin.dylib", "liblogos_blockchain_module_plugin.so", - "liblogos_blockchain_module_plugin.dll", - "circuits" + "liblogos_blockchain_module_plugin.dll" ], "nix": { diff --git a/src/logos_blockchain_module.cpp b/src/logos_blockchain_module.cpp index e6b9bf0..3c7a40b 100644 --- a/src/logos_blockchain_module.cpp +++ b/src/logos_blockchain_module.cpp @@ -122,6 +122,8 @@ namespace { std::string http_addr_data; std::string external_address_data; std::string state_path_data; + std::string storage_path_data; + std::string logs_path_data; bool ibd_val; std::string log_filter_data; std::string kms_file_data; @@ -197,6 +199,22 @@ namespace { ffi_args.state_path = nullptr; } + // storage_path (string -> const char*) — maps to storage.backend.folder_name + if (args.contains("storage_path") && args["storage_path"].is_string()) { + storage_path_data = args["storage_path"].get(); + ffi_args.storage_path = storage_path_data.c_str(); + } else { + ffi_args.storage_path = nullptr; + } + + // logs_path (string -> const char*) — maps to tracing.logger.file.directory + if (args.contains("logs_path") && args["logs_path"].is_string()) { + logs_path_data = args["logs_path"].get(); + ffi_args.logs_path = logs_path_data.c_str(); + } else { + ffi_args.logs_path = nullptr; + } + // ibd (bool -> const bool*) if (args.contains("ibd") && args["ibd"].is_boolean()) { ibd_val = args["ibd"].get(); @@ -224,37 +242,6 @@ namespace { }; } // namespace -namespace environment { - constexpr auto LOGOS_BLOCKCHAIN_CIRCUITS = "LOGOS_BLOCKCHAIN_CIRCUITS"; - - bool is_circuits_path_valid(const std::string& path) { - std::error_code ec; - if (!fs::is_directory(path, ec)) - return false; - for (const auto& entry : fs::directory_iterator(path, ec)) { // NOLINT(*-use-anyofallof) - if (entry.is_regular_file()) - return true; - } - return false; - } - - void setup_circuits_path(const std::string& module_path) { - const fs::path circuits_path = fs::path(module_path) / "circuits"; - const std::string circuits_str = circuits_path.string(); - - if (!is_circuits_path_valid(circuits_str)) { - fprintf( - stderr, - "FATAL: The LOGOS_BLOCKCHAIN_CIRCUITS environment variable is not set or does not contain any files.\n" - ); - return; - } - - setenv("LOGOS_BLOCKCHAIN_CIRCUITS", circuits_str.c_str(), 1); - fprintf(stderr, "LOGOS_BLOCKCHAIN_CIRCUITS set to: %s\n", circuits_str.c_str()); - } -} // namespace environment - void LogosBlockchainModule::on_new_block_callback(const char* block) { if (s_instance) { fprintf(stderr, "Received new block: %s\n", block); @@ -291,6 +278,75 @@ StdLogosResult LogosBlockchainModule::generate_user_config(const std::string& js return result::err(std::string("Failed to parse JSON args: ") + e.what()); } + // The module-context getters are populated by every logos-core host + // (logoscore-cli and Basecamp alike), so their mere presence can't tell the + // two apart. The bundled app therefore opts in explicitly by passing + // "use_persistence_paths": true; only then do we route the node's runtime + // directories — state, storage (db) and logs — under the host-owned + // per-instance persistence dir, so they all share one writable base. CLI and + // standalone callers omit the flag and keep their own paths (or the node + // defaults). Any path the caller set explicitly is left untouched. + bool use_persistence_paths = false; + if (const auto it = parsed_args.find("use_persistence_paths"); + it != parsed_args.end() && it->is_boolean()) { + use_persistence_paths = it->get(); + } + parsed_args.erase("use_persistence_paths"); // not an FFI field + + if (use_persistence_paths) { + const std::string& persistence = instancePersistencePath(); + if (!persistence.empty()) { + const fs::path base(persistence); + // Only fill a path the caller didn't pin (non-empty string wins). + const auto set_if_absent = [&parsed_args](const char* key, const std::string& value) { + const bool provided = parsed_args.contains(key) + && parsed_args[key].is_string() + && !parsed_args[key].get().empty(); + if (!provided) + parsed_args[key] = value; + }; + set_if_absent("state_path", (base / "state").string()); + set_if_absent("storage_path", (base / "db").string()); + set_if_absent("logs_path", (base / "logs").string()); + + // The config file itself is written under the same base, using the + // caller's path as the relative part below it ("config/user_config.yaml" + // → "/config/user_config.yaml"). Absolute / root-anchored inputs + // (e.g. "//user_config.yaml" from QDir::currentPath()=="/") are treated + // as relative to the base; a missing output defaults to + // "/user_config.yaml". + fs::path output_rel = "user_config.yaml"; + if (parsed_args.contains("output") && parsed_args["output"].is_string() + && !parsed_args["output"].get().empty()) { + const fs::path given(localPathFromFileUrl(parsed_args["output"].get())); + const fs::path rel = given.relative_path(); + output_rel = rel.empty() ? given.filename() : rel; + if (output_rel.empty()) + output_rel = "user_config.yaml"; + } + parsed_args["output"] = (base / output_rel).lexically_normal().string(); + + fprintf( + stderr, + "generate_user_config: routing output/state/storage/logs under instance persistence path: %s\n", + persistence.c_str() + ); + } else { + fprintf( + stderr, + "generate_user_config: use_persistence_paths requested but no instance persistence path is set; leaving paths unchanged.\n" + ); + } + } + + // The path the config is actually written to (after any persistence routing). + // Returned to the caller so it can hand the exact path to start(). Empty only + // when no output was given and no routing applied (the node wrote its own + // default relative to the cwd, which the module can't resolve). + std::string resolved_output; + if (parsed_args.contains("output") && parsed_args["output"].is_string()) + resolved_output = parsed_args["output"].get(); + const OwnedGenerateConfigArgs owned_args(parsed_args); const OperationStatus status = ::generate_user_config(owned_args.ffi_args); @@ -299,7 +355,7 @@ StdLogosResult LogosBlockchainModule::generate_user_config(const std::string& js return result::err("Failed to generate user config."); } - return result::ok(); + return result::ok(resolved_output); } StdLogosResult LogosBlockchainModule::start(const std::string& config_path, const std::string& deployment) { @@ -308,13 +364,6 @@ StdLogosResult LogosBlockchainModule::start(const std::string& config_path, cons return result::err("The node is already running."); } - const char* module_path_env = std::getenv("LOGOS_MODULE_PATH"); - if (module_path_env && *module_path_env) { - environment::setup_circuits_path(module_path_env); - } else { - fprintf(stderr, "Warning: LOGOS_MODULE_PATH not set, skipping circuits' path setup.\n"); - } - std::string effective_config_path = config_path; if (effective_config_path.empty()) { diff --git a/src/logos_blockchain_module.h b/src/logos_blockchain_module.h index df79054..d1959a4 100644 --- a/src/logos_blockchain_module.h +++ b/src/logos_blockchain_module.h @@ -26,7 +26,15 @@ public: [[nodiscard]] StdLogosResult stop(); // Config management - [[nodiscard]] static StdLogosResult generate_user_config(const std::string& json_args); + // Not static: when the JSON args set "use_persistence_paths": true it routes + // the node's output/state/storage/logs paths under instancePersistencePath() + // (config at /; state/db/logs at /state, /db, + // /logs), so it needs the instance context. Basecamp opts in via that flag; + // logoscore-cli/standalone omit it and keep their own paths. An explicit + // state/storage/logs path always wins; output is always re-anchored under the + // base when flagged. On success the result value is the path the config was + // written to — pass it straight to start(). + [[nodiscard]] StdLogosResult generate_user_config(const std::string& json_args); [[nodiscard]] static StdLogosResult update_user_config( const std::string& user_config_path, const std::string& keystore_path diff --git a/tests/mocks/mock_logos_blockchain.cpp b/tests/mocks/mock_logos_blockchain.cpp index 83b6c4b..233cfaa 100644 --- a/tests/mocks/mock_logos_blockchain.cpp +++ b/tests/mocks/mock_logos_blockchain.cpp @@ -6,6 +6,15 @@ #include #include #include +#include + +// Captures the paths passed to the most recent generate_user_config call so tests +// can assert the module's path routing. A null pointer is recorded as the +// sentinel "". +std::string g_lastGeneratedOutput; +std::string g_lastGeneratedStatePath; +std::string g_lastGeneratedStoragePath; +std::string g_lastGeneratedLogsPath; static char s_fakeNode = 0; static CryptarchiaInfo s_fakeCryptarchiaInfo = {}; @@ -26,6 +35,10 @@ bool is_ok(const OperationStatus* status) { OperationStatus generate_user_config(GenerateConfigArgs args) { LOGOS_CMOCK_RECORD("generate_user_config"); + g_lastGeneratedOutput = args.output ? args.output : ""; + g_lastGeneratedStatePath = args.state_path ? args.state_path : ""; + g_lastGeneratedStoragePath = args.storage_path ? args.storage_path : ""; + g_lastGeneratedLogsPath = args.logs_path ? args.logs_path : ""; return LOGOS_CMOCK_RETURN(int, "generate_user_config"); } diff --git a/tests/stubs/logos_blockchain.h b/tests/stubs/logos_blockchain.h index d8e5e6f..d9caee1 100644 --- a/tests/stubs/logos_blockchain.h +++ b/tests/stubs/logos_blockchain.h @@ -40,6 +40,8 @@ typedef struct { const char* http_addr; const char* external_address; const char* state_path; + const char* storage_path; + const char* logs_path; const bool* ibd; const char* log_filter; const char* kms_file; diff --git a/tests/test_blockchain.cpp b/tests/test_blockchain.cpp index 961d19c..24b2a0e 100644 --- a/tests/test_blockchain.cpp +++ b/tests/test_blockchain.cpp @@ -6,7 +6,6 @@ #include #include -#include #include #include #include @@ -40,16 +39,7 @@ struct TempDir { }; // Helper: create a module with a running (mocked) node. -// Sets up circuits directory, LOGOS_MODULE_PATH env, and calls start(). static LogosBlockchainModule* createStartedModule(LogosTestContext& t, TempDir& tmpDir) { - fs::create_directories(tmpDir.path / "circuits"); - { - std::ofstream f((tmpDir.path / "circuits" / "dummy.bin").string()); - f << "x"; - } - - setenv("LOGOS_MODULE_PATH", tmpDir.path.string().c_str(), 1); - auto* module = new LogosBlockchainModule(); t.mockCFunction("start_lb_node").returns(1); @@ -96,6 +86,122 @@ LOGOS_TEST(generate_user_config_from_json_string) { LOGOS_ASSERT(t.cFunctionCalled("generate_user_config")); } +// The mock records the paths handed to the FFI (see mock_logos_blockchain.cpp). +extern std::string g_lastGeneratedOutput; +extern std::string g_lastGeneratedStatePath; +extern std::string g_lastGeneratedStoragePath; +extern std::string g_lastGeneratedLogsPath; + +static void clearGeneratedPaths() { + g_lastGeneratedOutput.clear(); + g_lastGeneratedStatePath.clear(); + g_lastGeneratedStoragePath.clear(); + g_lastGeneratedLogsPath.clear(); +} + +// Basecamp opts in with the flag: output and state/storage/logs are routed under +// the per-instance persistence dir. The output keeps its given path as the +// relative part below the base. +LOGOS_TEST(generate_user_config_routes_paths_under_persistence_when_flagged) { + auto t = LogosTestContext("blockchain_module"); + LogosBlockchainModule module; + module._logosCoreSetContext_("/mod", "id", "/persist/data"); + + t.mockCFunction("generate_user_config").returns(0); + clearGeneratedPaths(); + + StdLogosResult result = + module.generate_user_config(R"({"output":"config/user_config.yaml","use_persistence_paths":true})"); + LOGOS_ASSERT_TRUE(result.success); + // The resolved config path is returned for the caller to hand to start(). + LOGOS_ASSERT_EQ(result.value.get(), std::string("/persist/data/config/user_config.yaml")); + LOGOS_ASSERT_EQ(g_lastGeneratedOutput, std::string("/persist/data/config/user_config.yaml")); + LOGOS_ASSERT_EQ(g_lastGeneratedStatePath, std::string("/persist/data/state")); + LOGOS_ASSERT_EQ(g_lastGeneratedStoragePath, std::string("/persist/data/db")); + LOGOS_ASSERT_EQ(g_lastGeneratedLogsPath, std::string("/persist/data/logs")); +} + +// A root-anchored output (the UI's "//user_config.yaml" artifact) is treated as +// relative to the base, not written to the read-only filesystem root. +LOGOS_TEST(generate_user_config_routes_root_anchored_output_under_persistence) { + auto t = LogosTestContext("blockchain_module"); + LogosBlockchainModule module; + module._logosCoreSetContext_("/mod", "id", "/persist/data"); + + t.mockCFunction("generate_user_config").returns(0); + clearGeneratedPaths(); + + LOGOS_ASSERT_TRUE( + module.generate_user_config(R"({"output":"//user_config.yaml","use_persistence_paths":true})").success); + LOGOS_ASSERT_EQ(g_lastGeneratedOutput, std::string("/persist/data/user_config.yaml")); +} + +// With no output given, the config defaults to /user_config.yaml. +LOGOS_TEST(generate_user_config_defaults_output_under_persistence) { + auto t = LogosTestContext("blockchain_module"); + LogosBlockchainModule module; + module._logosCoreSetContext_("/mod", "id", "/persist/data"); + + t.mockCFunction("generate_user_config").returns(0); + clearGeneratedPaths(); + + LOGOS_ASSERT_TRUE(module.generate_user_config(R"({"use_persistence_paths":true})").success); + LOGOS_ASSERT_EQ(g_lastGeneratedOutput, std::string("/persist/data/user_config.yaml")); +} + +// A host load WITHOUT the flag (e.g. logoscore-cli) does NOT redirect any path, +// even though the persistence path is populated. +LOGOS_TEST(generate_user_config_host_without_flag_leaves_paths_unset) { + auto t = LogosTestContext("blockchain_module"); + LogosBlockchainModule module; + module._logosCoreSetContext_("/mod", "id", "/persist/data"); + + t.mockCFunction("generate_user_config").returns(0); + clearGeneratedPaths(); + + StdLogosResult result = module.generate_user_config(R"({"output":"/tmp/out.json"})"); + LOGOS_ASSERT_TRUE(result.success); + // Without routing, the caller's output is returned unchanged. + LOGOS_ASSERT_EQ(result.value.get(), std::string("/tmp/out.json")); + LOGOS_ASSERT_EQ(g_lastGeneratedOutput, std::string("/tmp/out.json")); + LOGOS_ASSERT_EQ(g_lastGeneratedStatePath, std::string("")); + LOGOS_ASSERT_EQ(g_lastGeneratedStoragePath, std::string("")); + LOGOS_ASSERT_EQ(g_lastGeneratedLogsPath, std::string("")); +} + +// An explicitly provided path wins over the flag; the others are still routed. +LOGOS_TEST(generate_user_config_explicit_path_wins_over_flag) { + auto t = LogosTestContext("blockchain_module"); + LogosBlockchainModule module; + module._logosCoreSetContext_("/mod", "id", "/persist/data"); + + t.mockCFunction("generate_user_config").returns(0); + clearGeneratedPaths(); + + LOGOS_ASSERT_TRUE( + module.generate_user_config( + R"({"output":"/tmp/out.json","state_path":"/tmp/state","use_persistence_paths":true})").success); + LOGOS_ASSERT_EQ(g_lastGeneratedStatePath, std::string("/tmp/state")); + LOGOS_ASSERT_EQ(g_lastGeneratedStoragePath, std::string("/persist/data/db")); + LOGOS_ASSERT_EQ(g_lastGeneratedLogsPath, std::string("/persist/data/logs")); +} + +// The flag with no host context (standalone) leaves paths unset. +LOGOS_TEST(generate_user_config_flag_without_context_leaves_paths_unset) { + auto t = LogosTestContext("blockchain_module"); + LogosBlockchainModule module; + + t.mockCFunction("generate_user_config").returns(0); + clearGeneratedPaths(); + + LOGOS_ASSERT_TRUE( + module.generate_user_config(R"({"output":"/tmp/out.json","use_persistence_paths":true})").success); + LOGOS_ASSERT_EQ(g_lastGeneratedOutput, std::string("/tmp/out.json")); + LOGOS_ASSERT_EQ(g_lastGeneratedStatePath, std::string("")); + LOGOS_ASSERT_EQ(g_lastGeneratedStoragePath, std::string("")); + LOGOS_ASSERT_EQ(g_lastGeneratedLogsPath, std::string("")); +} + LOGOS_TEST(generate_user_config_with_all_fields) { auto t = LogosTestContext("blockchain_module"); LogosBlockchainModule module;