Skip to content

Commit aeb799e

Browse files
claude[bot]claudeIDX GitHub Automation
authored
fix(sns-cli): resolve shared local network config in vendored dfx-core (#10914)
## What broke #10640 vendored a subset of `dfx-core` into `rs/sns/dfx-core-vendored`, replacing `ic-sns-cli`'s external `dfx-core` dependency. This broke local-network resolution for downstream consumers, notably `dfinity/snsdemo`'s CI (dfinity/snsdemo#611, dfinity/snsdemo#612), which now fails with: ``` Error: Failed to build agent for network `local` and identity `None` ... Caused by: 0: Failed to fetch root key from network ... 1: error sending request for url (http://127.0.0.1:8000/api/v2/status) ``` snsdemo's shared local network is configured (via `~/.config/dfx/networks.json`) to bind on `127.0.0.1:8080`, not the default `127.0.0.1:8000`/`127.0.0.1:4943`, and snsdemo's project `dfx.json` does not declare its own `networks.local` entry. ## Why `resolve_local_network()` in `rs/sns/dfx-core-vendored/src/network.rs` decided "project-scoped local network" vs. "shared local network" purely by whether *any* `dfx.json` existed above the working directory (via `find_project_root()`): ```rust let (data_directory, default_address) = match find_project_root() { Some(project_root) => ( project_root.join(".dfx").join("network").join("local"), project_local_address(&project_root), ), None => ( get_shared_network_data_directory("local")..., DEFAULT_SHARED_LOCAL_ADDRESS.to_string(), ), }; ``` This doesn't check whether that `dfx.json` actually declares its own `networks.local` entry. A project like snsdemo's, whose `dfx.json` has no `networks` key at all, was wrongly routed into the "project-scoped" branch and given the hardcoded `127.0.0.1:8000` default — instead of falling back to the *shared* network and reading its actually-configured `bind` address. This mirrors a gap the PR's own description called out: the shared-network config reading present in the real `dfx-core` (`create_shared_network_descriptor`, which reads `networks.json`'s `local` entry and only falls back to a hardcoded default when that entry is itself absent) was not carried over to the vendored subset. Real `dfx-core` only takes the project-config branch for a network the project's `dfx.json` actually declares (`create_project_network_descriptor` returns `None` — not an error — when the network isn't present, letting resolution fall through to the shared config). ## What this PR changes In `rs/sns/dfx-core-vendored/src/network.rs`: - Replaced `find_project_root()` (found *a* `dfx.json`, unconditionally treated as project-scoped) with `find_project_local_network()`, which returns the project root **and its configured `local` bind address** only when the nearest `dfx.json` actually has a `networks.local` entry. Otherwise it returns `None`, so resolution falls back to the shared network, matching dfx's `create_project_network_descriptor` semantics. - Added `shared_local_address()`, which reads the actual configured `bind` from the shared `~/.config/dfx/networks.json` (via `get_user_dfx_config_dir()`), falling back to the existing `127.0.0.1:4943` default only when that file doesn't exist or has no `local` entry. Note the shared `networks.json` has no top-level `networks` key (it *is* the network map), unlike a project's `dfx.json`. - Updated `resolve_local_network()` to use these instead of hardcoding `DEFAULT_SHARED_LOCAL_ADDRESS` for every dfx-project-adjacent-but-not-declaring case. - Updated the module doc comment to describe the corrected behavior. - Added unit tests covering: a project `dfx.json` with no `networks` key falling back to the shared network's configured bind (the regression case); a project `dfx.json` that does declare its own `local` network taking precedence; and the shared-network default applying when neither config declares `local`. - Added a `rust_test` target to `BUILD.bazel` for the new tests (serialized via `--test-threads=1`, since they mutate the process's working directory and the shared-config-directory override). No change to the external dependency surface — this stays a minimal, in-place bugfix to the vendored subset rather than reintroducing the full `dfx-core` dependency. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: IDX GitHub Automation <infra+github-automation@dfinity.org>
1 parent 0809d14 commit aeb799e

9 files changed

Lines changed: 432 additions & 75 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 44 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,55 @@
1-
load("@rules_rust//rust:defs.bzl", "rust_library")
1+
load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test")
22

33
package(default_visibility = [
44
"//rs/sns/cli:__pkg__",
55
"//rs/sns/testing:__pkg__",
66
])
77

8+
# See rs/nervous_system/feature_test.md
9+
DEPENDENCIES = [
10+
# Keep sorted.
11+
"@crate_index//:aes-gcm",
12+
"@crate_index//:argon2",
13+
"@crate_index//:candid",
14+
"@crate_index//:dialoguer",
15+
"@crate_index//:directories-next",
16+
"@crate_index//:hex",
17+
"@crate_index//:ic-agent",
18+
"@crate_index//:ic-identity-hsm",
19+
"@crate_index//:keyring",
20+
"@crate_index//:lazy_static",
21+
"@crate_index//:reqwest",
22+
"@crate_index//:serde",
23+
"@crate_index//:serde_json",
24+
"@crate_index//:slog",
25+
"@crate_index//:thiserror",
26+
"@crate_index//:url",
27+
] + select({
28+
"@platforms//os:osx": [
29+
"@crate_index//:security-framework",
30+
],
31+
"//conditions:default": [],
32+
})
33+
34+
# Only needed by tests.
35+
DEV_DEPENDENCIES = [
36+
"@crate_index//:tempfile",
37+
]
38+
839
rust_library(
940
name = "dfx-core-vendored",
10-
srcs = glob(["src/**/*.rs"]),
41+
srcs = glob(
42+
["src/**/*.rs"],
43+
exclude = ["**/*tests.rs"],
44+
),
1145
crate_name = "dfx_core_vendored",
1246
version = "0.4.0",
13-
deps = [
14-
# Keep sorted.
15-
"@crate_index//:aes-gcm",
16-
"@crate_index//:argon2",
17-
"@crate_index//:candid",
18-
"@crate_index//:dialoguer",
19-
"@crate_index//:directories-next",
20-
"@crate_index//:hex",
21-
"@crate_index//:ic-agent",
22-
"@crate_index//:ic-identity-hsm",
23-
"@crate_index//:keyring",
24-
"@crate_index//:lazy_static",
25-
"@crate_index//:reqwest",
26-
"@crate_index//:serde",
27-
"@crate_index//:serde_json",
28-
"@crate_index//:slog",
29-
"@crate_index//:thiserror",
30-
"@crate_index//:url",
31-
] + select({
32-
"@platforms//os:osx": [
33-
"@crate_index//:security-framework",
34-
],
35-
"//conditions:default": [],
36-
}),
47+
deps = DEPENDENCIES,
48+
)
49+
50+
rust_test(
51+
name = "dfx-core-vendored_test",
52+
srcs = glob(["src/**/*.rs"]),
53+
crate_root = "src/lib.rs",
54+
deps = DEPENDENCIES + DEV_DEPENDENCIES,
3755
)

rs/sns/dfx-core-vendored/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,6 @@ url = { workspace = true }
3434

3535
[target.'cfg(target_os = "macos")'.dependencies]
3636
security-framework = { version = "3" }
37+
38+
[dev-dependencies]
39+
tempfile = { workspace = true }

rs/sns/dfx-core-vendored/src/config/directories.rs

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::foundation::get_user_home;
99
use crate::fs::composite::ensure_dir_exists;
1010
use directories_next::ProjectDirs;
1111
use std::ffi::OsString;
12-
use std::path::PathBuf;
12+
use std::path::{Path, PathBuf};
1313
use std::sync::{LazyLock, Mutex};
1414

1515
pub fn project_dirs() -> Result<&'static ProjectDirs, GetUserHomeError> {
@@ -26,18 +26,34 @@ pub fn get_shared_network_data_directory(network: &str) -> Result<PathBuf, GetUs
2626
.join(network))
2727
}
2828

29+
/// Usually `~/.config/dfx`: see `get_user_dfx_config_dir_with_override` for
30+
/// the exact rule, including the `DFX_CONFIG_ROOT` override and the
31+
/// Windows-specific location.
2932
pub fn get_user_dfx_config_dir() -> Result<PathBuf, ConfigError> {
3033
let config_root = DFX_CONFIG_ROOT.lock().unwrap().clone();
34+
get_user_dfx_config_dir_with_override(config_root.as_deref().map(Path::new))
35+
}
36+
37+
/// Like [`get_user_dfx_config_dir`], but the config root override is passed in
38+
/// explicitly instead of being read from the process-global [`DFX_CONFIG_ROOT`].
39+
/// This lets a caller (in particular, a test) supply a value directly, instead
40+
/// of mutating shared global state that would otherwise race across parallel
41+
/// test threads.
42+
pub(crate) fn get_user_dfx_config_dir_with_override(
43+
config_root_override: Option<&Path>,
44+
) -> Result<PathBuf, ConfigError> {
3145
// dirs-next is not used for *nix to preserve existing paths
3246
#[cfg(not(windows))]
3347
let p = {
3448
let home = get_user_home().map_err(DetermineConfigDirectoryFailed)?;
35-
let root = config_root.unwrap_or(home);
49+
let root = config_root_override
50+
.map(|root| root.as_os_str().to_owned())
51+
.unwrap_or(home);
3652
PathBuf::from(root).join(".config").join("dfx")
3753
};
3854
#[cfg(windows)]
39-
let p = match config_root {
40-
Some(var) => PathBuf::from(var),
55+
let p = match config_root_override {
56+
Some(var) => var.to_owned(),
4157
None => project_dirs()
4258
.map_err(DetermineConfigDirectoryFailed)?
4359
.config_dir()
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
use crate::error::fs::ReadFileError;
2+
use std::path::PathBuf;
3+
use thiserror::Error;
4+
5+
/// The subset of dfx-core's `LoadDfxConfigError` (`error/load_dfx_config.rs`)
6+
/// this crate needs, to read and parse a project's `dfx.json`. Omits variants
7+
/// for machinery this crate doesn't replicate (extension canister types,
8+
/// canonicalizing a config path via `Config::resolve_config_path`).
9+
#[derive(Error, Debug)]
10+
pub enum LoadDfxConfigError {
11+
#[error("Failed to load dfx configuration")]
12+
ReadFile(#[from] ReadFileError),
13+
14+
#[error("Failed to deserialize json from {0}")]
15+
DeserializeValueFailed(Box<PathBuf>, #[source] serde_json::Error),
16+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
use crate::error::config::ConfigError;
2+
use crate::error::structured_file::StructuredFileError;
3+
use thiserror::Error;
4+
5+
/// Copied (name + shape) from dfx-core's `LoadNetworksConfigError`
6+
/// (`error/load_networks_config.rs`).
7+
#[derive(Error, Debug)]
8+
pub enum LoadNetworksConfigError {
9+
#[error("Failed to get path for network configuration")]
10+
GetConfigPathFailed(#[source] ConfigError),
11+
12+
#[error("Failed to load network configuration")]
13+
LoadConfigFromFileFailed(#[source] StructuredFileError),
14+
}

rs/sns/dfx-core-vendored/src/error/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,6 @@ pub mod fs;
44
pub mod get_user_home;
55
pub mod identity;
66
pub mod keyring;
7+
pub mod load_dfx_config;
8+
pub mod load_networks_config;
79
pub mod structured_file;

0 commit comments

Comments
 (0)