Skip to content

chore: vendor dfx-core identity/network resolution, drop external dfx-core dependency#10640

Open
lwshang wants to merge 4 commits into
masterfrom
linwei/drop-dfx-core-dep
Open

chore: vendor dfx-core identity/network resolution, drop external dfx-core dependency#10640
lwshang wants to merge 4 commits into
masterfrom
linwei/drop-dfx-core-dep

Conversation

@lwshang

@lwshang lwshang commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Why

ic-sns-cli and ic-sns-testing depended on the external
dfx-core crate. dfx-core re-exports
ic-agent in its public API, so its major version is bumped in lockstep with
ic-agent
— every ic-agent upgrade in this repo requires a matching
dfx-core release.

Once the dfinity/sdk repo is archived, those
dfx-core releases can no longer be cut, which would block this monorepo from
upgrading ic-agent at all. It also drags in a pile of outdated transitive deps
(bip32 0.4, tiny-bip39, sec1 0.3, handlebars, byte-unit, …) that other
teams have hit while trying to upgrade their own dependencies.

SNS only uses a small, self-contained slice of dfx-core: resolving a dfx
identity + network into an ic_agent::Agent, and resolving an identity name to
a principal. This PR vendors just that slice as a new internal crate and removes
the external dfx-core dependency.

This direction was approved by the NNS team, who own ic-sns-cli: .

What

  • New crate rs/sns/dfx-core-vendored — a trimmed, in-tree vendoring of
    dfx-core 0.3.0 exposing two functions:
    • get_agent(network, identity) -> Agent
    • get_identity_principal(name) -> Principal
  • ic-sns-cli and ic-sns-testing rewired onto it; the dfx-core dependency
    removed from both crates and from the workspace (Cargo.toml,
    bazel/rust.MODULE.bazel, lockfiles).
  • The keyring/encrypted-PEM deps dfx-core used to pull in
    (aes-gcm, argon2, dialoguer, keyring, security-framework) become
    direct deps of the new crate; the unused ones (bip32 0.4, tiny-bip39,
    sec1 0.3, handlebars, byte-unit, dunce, tar, flate2, …) drop out of
    the graph.

Behaviour

No user-facing change. dfx identities are untouched (~/.config/dfx/identity/…)
and the SNS CLI behaves as before for the supported cases:

  • Identities: plaintext PEM, password-encrypted PEM, OS keyring, and HSM —
    all still load exactly as dfx loads them.
  • Networks: ic (mainnet), local (shared or project, honouring a running
    replica's webserver-port), and explicit IC HTTP endpoint URLs (e.g.
    sns-testing --network http://127.0.0.1:8080).

Not vendored (unused by SNS): identity creation/rename/removal/export, wallets,
the extension manager, playground networks, and general dfx.json/networks.json
parsing beyond the local bind. IdentityManager::new no longer auto-creates a
default identity for InitializeIdentity::Allow (that is dfx's job), but SNS only
ever calls it with Disallow, so there is no behavioural change in practice.

See rs/sns/dfx-core-vendored/README.md for a per-module diff against
dfx-core 0.3.0 and the maintenance expectations (the crate is treated as frozen
vendored code — the only expected changes are the mechanical adjustments to track
future ic-agent majors, which is the whole reason it exists).

Testing

  • cargo build + cargo clippy (-D warnings, per ci/scripts/rust-lint.sh)
    on dfx-core-vendored, ic-sns-cli, ic-sns-testing.
  • bazel build of the new crate and all //rs/sns/cli/... + //rs/sns/testing/...
    targets.
  • bazel test:
    • //rs/sns/cli:sns_test, //rs/sns/cli:sns_doctest,
      //rs/sns/testing:sns_testing_test
    • //rs/sns/testing:sns_testing_ci (full PocketIC NNS + SNS lifecycle)
    • //rs/nervous_system/integration_tests:sns_extension_test
  • After merging the latest master, the crate compiles cleanly against the
    upgraded ic-agent / ic-identity-hsm 0.47, with no dfx-core release
    required — a live confirmation of the decoupling.

lwshang and others added 2 commits July 2, 2026 15:51
…-core dep

ic-sns-cli and ic-sns-testing depended on the external dfx-core crate, which
re-exports ic-agent in its public API and so bumps its major version in lockstep
with ic-agent. Once the dfinity/sdk repo is archived those releases can no longer
be cut, which would block this monorepo from upgrading ic-agent.

SNS only uses a small, self-contained slice of dfx-core: resolving a dfx
identity + network into an ic_agent::Agent, and resolving an identity name to a
principal. This vendors that slice as a new internal crate,
rs/sns/dfx-core-vendored, and removes the external dfx-core dependency.

Behaviour is preserved for the supported cases: dfx identities keep working
(plaintext/encrypted PEM, OS keyring, and HSM), and network resolution covers
ic, local (honouring a running replica's webserver-port), and IC HTTP endpoint
URLs. Identity creation, wallets, extensions, playground, and general
dfx.json/networks.json parsing are not vendored (unused by SNS). See
rs/sns/dfx-core-vendored/README.md for a per-module diff against dfx-core 0.3.0.

Removing dfx-core also drops the outdated transitive deps it pulled in
(bip32 0.4.0, tiny-bip39, sec1 0.3, handlebars, byte-unit, ...).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts:
#	Cargo.Bazel.json.lock
#	Cargo.Bazel.toml.lock
#	Cargo.lock
#	Cargo.toml
#	bazel/rust.MODULE.bazel

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the workspace’s external dependency on dfx-core by introducing an in-tree, trimmed vendoring (rs/sns/dfx-core-vendored) that provides only the SNS tooling functionality needed to (1) resolve a dfx identity/network into an ic_agent::Agent and (2) resolve an identity name to its principal, thereby decoupling ic-agent upgrades from dfx-core release cadence.

Changes:

  • Added new internal crate dfx-core-vendored with minimal identity loading + ic/local/URL network resolution and agent construction.
  • Rewired ic-sns-cli and ic-sns-testing to use dfx-core-vendored and removed dfx-core from Cargo/Bazel deps and lockfiles.
  • Updated Bazel module crate specs to reflect the new direct dependency set pulled in by the vendored subset.

Reviewed changes

Copilot reviewed 32 out of 35 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
rs/sns/testing/src/utils.rs Switch identity principal resolution to dfx-core-vendored and improve error context.
rs/sns/testing/Cargo.toml Replace dfx-core dependency with path dep on dfx-core-vendored.
rs/sns/testing/BUILD.bazel Replace @crate_index//:dfx-core with //rs/sns/dfx-core-vendored.
rs/sns/dfx-core-vendored/src/network.rs New minimal network resolution implementation for ic/local/URL.
rs/sns/dfx-core-vendored/src/lib.rs New public API (get_agent, get_identity_principal) and agent construction wiring.
rs/sns/dfx-core-vendored/src/json.rs Vendored JSON file load helper used by identity/keyring paths.
rs/sns/dfx-core-vendored/src/identity/pem_safekeeping.rs Vendored PEM loading/decrypting path (plaintext/encrypted/keyring).
rs/sns/dfx-core-vendored/src/identity/mod.rs Vendored identity types and ic_agent::Identity adapter.
rs/sns/dfx-core-vendored/src/identity/keyring_mock.rs Vendored keyring load path + CI mock hook support.
rs/sns/dfx-core-vendored/src/identity/identity_manager.rs Vendored identity selection/instantiation against on-disk dfx identity store.
rs/sns/dfx-core-vendored/src/identity/identity_file_locations.rs Vendored identity file location helpers.
rs/sns/dfx-core-vendored/src/fs/mod.rs Vendored read-side filesystem helpers.
rs/sns/dfx-core-vendored/src/fs/composite.rs Vendored ensure_dir_exists helper.
rs/sns/dfx-core-vendored/src/foundation.rs Vendored HOME resolution helper.
rs/sns/dfx-core-vendored/src/error/structured_file.rs Vendored structured-file read errors (JSON read/parse).
rs/sns/dfx-core-vendored/src/error/mod.rs New reduced error module surface for the vendored subset.
rs/sns/dfx-core-vendored/src/error/keyring.rs Vendored keyring errors reachable via the load path.
rs/sns/dfx-core-vendored/src/error/identity.rs Vendored identity loading/instantiation error types.
rs/sns/dfx-core-vendored/src/error/get_user_home.rs Vendored HOME-missing error type.
rs/sns/dfx-core-vendored/src/error/fs.rs Vendored filesystem error wrappers used by read helpers.
rs/sns/dfx-core-vendored/src/error/encryption.rs Vendored decrypt-path encryption errors.
rs/sns/dfx-core-vendored/src/error/config.rs Vendored config directory resolution errors.
rs/sns/dfx-core-vendored/src/config/mod.rs New minimal config module root.
rs/sns/dfx-core-vendored/src/config/directories.rs Vendored dfx config dir + shared network dir resolution.
rs/sns/dfx-core-vendored/README.md Documentation of vendoring scope, provenance, and maintenance expectations.
rs/sns/dfx-core-vendored/Cargo.toml New crate manifest + direct deps replacing transitive dfx-core deps.
rs/sns/dfx-core-vendored/BUILD.bazel New Bazel target for the vendored crate + deps.
rs/sns/cli/src/utils.rs Replace DfxInterface usage with dfx-core-vendored::get_agent.
rs/sns/cli/Cargo.toml Replace dfx-core dependency with path dep on dfx-core-vendored.
rs/sns/cli/BUILD.bazel Replace @crate_index//:dfx-core with //rs/sns/dfx-core-vendored.
Cargo.toml Add rs/sns/dfx-core-vendored to workspace members and remove dfx-core from deps list.
Cargo.lock Remove dfx-core and reflect new dependency graph rooted at dfx-core-vendored.
Cargo.Bazel.toml.lock Update Bazel cargo lock to remove dfx-core and add new direct deps.
Cargo.Bazel.json.lock Update Bazel cargo lock JSON to remove dfx-core and add new direct deps.
bazel/rust.MODULE.bazel Remove dfx-core crate spec and add direct crate specs (e.g., keyring, dialoguer, etc.).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread rs/sns/dfx-core-vendored/src/network.rs Outdated
Comment thread rs/sns/dfx-core-vendored/src/identity/pem_safekeeping.rs Outdated
@lwshang lwshang marked this pull request as ready for review July 2, 2026 17:53
@lwshang lwshang requested review from a team as code owners July 2, 2026 17:53

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):

  1. Update unreleased_changelog.md (if there are behavior changes, even if they are
    non-breaking).

  2. Are there BREAKING changes?

  3. Is a data migration needed?

  4. Security review?

How to Satisfy This Automatic Review

  1. Go to the bottom of the pull request page.

  2. Look for where it says this bot is requesting changes.

  3. Click the three dots to the right.

  4. Select "Dismiss review".

  5. In the text entry box, respond to each of the numbered items in the previous
    section, declare one of the following:

  • Done.

  • $REASON_WHY_NO_NEED. E.g. for unreleased_changelog.md, "No
    canister behavior changes.", or for item 2, "Existing APIs
    behave as before.".

Brief Guide to "Externally Visible" Changes

"Externally visible behavior change" is very often due to some NEW canister API.

Changes to EXISTING APIs are more likely to be "breaking".

If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.

If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.

Reference(s)

For a more comprehensive checklist, see here.

GOVERNANCE_CHECKLIST_REMINDER_DEDUP

@github-actions github-actions Bot added the @idx label Jul 2, 2026
@zeropath-ai

zeropath-ai Bot commented Jul 2, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 4a403fe.

Security Overview
Detected Code Changes

The diff is too large to display a summary of code changes.

@lwshang lwshang dismissed github-actions[bot]’s stale review July 2, 2026 17:56

No canister behavior changes.
Existing APIs behave as before.

lwshang and others added 2 commits July 2, 2026 19:24
- network: drop the unreachable ParseProviderUrlFailed error variant and the
  parse_provider_url wrapper; use Url::parse(..).is_ok() directly. Behaviour is
  unchanged and still mirrors dfx-core's create_url_based_network_descriptor (an
  unparseable identifier falls through to NetworkNotFound).
- pem_safekeeping: fix inherited doc-comment typo IndentityConfiguration.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants