chore: bump agent-rs to 0.48 git rev for rustls provider feature#4539
Draft
lwshang wants to merge 1 commit into
Draft
chore: bump agent-rs to 0.48 git rev for rustls provider feature#4539lwshang wants to merge 1 commit into
lwshang wants to merge 1 commit into
Conversation
PR 732 in dfinity/agent-rs introduced `tls-aws-lc-rs` and `tls-ring` cargo features to let consumers pick the rustls crypto provider. Workspace `ic-agent` is now pinned to that HEAD with `default-features = false, features = ["pem"]`. Crates that build an `Agent` via ic-agent's default reqwest client (dfx, icx-asset) opt into `tls-aws-lc-rs`. dfx-core uses `with_http_client`, so no provider is needed on its path and no feature plumbing is added there. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ic-agent,ic-utils,ic-identity-hsmto HEAD of agent-rsmain(01b6b2c, == 0.48.0), picking up dfinity/agent-rs#732 which addstls-aws-lc-rs(default) andtls-ringcargo features for rustls crypto provider selection.ic-agentdeclared withdefault-features = false, features = ["pem"]so each crate opts into its own TLS feature.dfxandicx-assetenabletls-aws-lc-rsbecause both callAgent::builder()with the default reqwest client (where ic-agent now needs a provider installed, else panics with "No provider set").dfx-coredoes not add any TLS feature plumbing. Itsbuild_agentsupplies its own client viawith_http_client, on which ic-agent installs no provider, sopemalone is enough.Investigation notes
Should dfx-core forward TLS features?
No. Two reasons:
with_http_clientshort-circuits the provider install.pub use ic_agent, so any downstream consumer who callsAgent::builder()already has their ownic-agentdep and picks their TLS feature there.Should workspace
reqwestswitch torustls-no-provider?No. PR 732 made that switch in ic-agent because rustls 0.23 unification with both
ringandaws_lc_rsfeatures active panics when picking a default provider. Our workspace doesn't hit that — the only other reqwest is 0.12.28 (viapocket-ic) with no TLS features enabled. Switching would also require us toCryptoProvider::install_default(...)before eachClient::builder().use_rustls_tls()call in dfx-core, util/mod.rs, and project/import.rs — added complexity for no concrete benefit.A cleaner refactor (drop the manual
use_rustls_tls()calls, let ic-agent build its own default client) could centralize TLS choice in ic-agent's features, but is left as a possible follow-up.Test plan
cargo check --workspacepassescargo check -p dfx-corepasses (no other workspace crate pulls intls-aws-lc-rs)cargo check -p dfx-core --no-default-features --features tls-ring— N/A here (no dfx-core features added); but verified during investigationdfxagainst mainnet / local replica to confirm TLS still works at runtime🤖 Generated with Claude Code