fix(grpc): set tls_config on manual endpoint connect

[Jeezman]

#229 switched Client::connect from the generated Client::connect(url) static to building the channel manually via Endpoint::from_shared(url).connect(), but the manual path does not apply TLS configuration. In tonic 0.14 the tls-webpki-roots / tls-native-roots feature flags pull in rustls without auto-configuring a ClientTlsConfig, so HTTPS Ark servers became unreachable in v0.9.1 with a generic transport error.

Apply ClientTlsConfig with webpki or native roots based on the enabled feature flag, gated so non-TLS builds remain unaffected.

Resolves #232.

Summary by CodeRabbit

• Bug Fixes
  • Improved gRPC client connection reliability by explicitly establishing the transport channel and surfacing clearer connection errors.
  • Added conditional TLS handling to support different root certificate configurations, reducing connection failures in secured environments.
  • Enhanced robustness during initial connection setup to improve overall startup stability.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 60c29bc5-c936-4cd5-9943-3214759b1098

📥 Commits

Reviewing files that changed from the base of the PR and between 100c53a and 2e9bbb3.

📒 Files selected for processing (1)

• ark-grpc/src/client.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• ark-grpc/src/client.rs

---

Walkthrough

Client::connect() restores TLS configuration for HTTPS endpoints by building an Endpoint, conditionally applying TLS roots under feature flags, then explicitly connecting. Connection errors are mapped via Error::connect.

Changes

gRPC Client TLS Configuration Restoration

Layer / File(s)	Summary
Endpoint creation with conditional TLS configuration ark-grpc/src/client.rs	Client::connect now creates a tonic::transport::Endpoint from the URL, conditionally configures TLS roots when compiled with tls-webpki-roots or tls-native-roots, then calls connect().await to obtain the channel with errors mapped through Error::connect.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(grpc): set tls_config on manual endpoint connect' accurately describes the main change: applying TLS configuration to the manual endpoint connection path in the gRPC client.
Linked Issues check	✅ Passed	The PR directly addresses issue #232 by restoring TLS configuration for manual gRPC channel construction, applying ClientTlsConfig conditionally based on feature flags (tls-webpki-roots/tls-native-roots).
Out of Scope Changes check	✅ Passed	All changes are scoped to Client::connect() in ark-grpc/src/client.rs and directly address the TLS configuration regression, with no unrelated modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[arkanaai]

✅ Approved

Clean, well-scoped fix for the TLS regression introduced in #229. Reviewed the full diff and surrounding client.rs context.

What it does

PR #229 switched from ArkServiceClient::connect(url) (which auto-configured TLS via feature flags) to manual Endpoint::from_shared(url).connect(), which does not auto-apply ClientTlsConfig. This PR correctly bridges that gap by explicitly building and applying a ClientTlsConfig when TLS features are enabled.

Verified

• Feature gating is correct. The TLS block is behind #[cfg(any(feature = "tls-webpki-roots", feature = "tls-native-roots"))], so non-TLS builds are unaffected.
• Error handling is correct. tls_config() returns Result and is properly propagated via map_err(Error::connect)?.
• No public API change. connect(&mut self) -> Result<(), Error> signature is unchanged — zero cross-repo impact. Downstream consumers (ark-client, ark-rs, e2e-tests, ark-client-sample) are unaffected.
• Feature forwarding is consistent. ark-client/Cargo.toml and ark-rs/Cargo.toml both forward tls-webpki-roots / tls-native-roots to ark-grpc, so the right #[cfg] paths activate.
• Default features (tls-webpki-roots) mean the common case — cargo build with defaults — gets webpki roots applied, which is the correct fix for the reported issue.
• Not protocol-critical. This is transport-layer plumbing; no VTXO, signing, forfeit, or round-lifecycle code is touched.

Nit (non-blocking)

ark-grpc/src/client.rs — dual-feature edge case. If a consumer enables both tls-webpki-roots and tls-native-roots, both with_webpki_roots() and with_native_roots() are called sequentially. In tonic 0.14, the second call likely overrides the first (native roots win). This is harmless in practice — Cargo feature flags are additive and enabling both is unusual — but a #[cfg(all(...))] compile error or a doc comment noting the precedence would prevent future confusion. Not a blocker.

LGTM. Ship it.

[arkanaai]

✅ Approved

Clean, well-scoped fix for the TLS regression introduced in #229. Reviewed the full diff and surrounding client.rs context.

What it does

PR #229 switched from ArkServiceClient::connect(url) (which auto-configured TLS via feature flags) to manual Endpoint::from_shared(url).connect(), which does not auto-apply ClientTlsConfig. This PR correctly bridges that gap by explicitly building and applying a ClientTlsConfig when TLS features are enabled.

Verified

• Feature gating is correct. The TLS block is behind #[cfg(any(feature = "tls-webpki-roots", feature = "tls-native-roots"))], so non-TLS builds are unaffected.
• Error handling is correct. tls_config() returns Result and is properly propagated via map_err(Error::connect)?.
• No public API change. connect(&mut self) -> Result<(), Error> signature is unchanged — zero cross-repo impact. Downstream consumers (ark-client, ark-rs, e2e-tests, ark-client-sample) are unaffected.
• Feature forwarding is consistent. ark-client/Cargo.toml and ark-rs/Cargo.toml both forward tls-webpki-roots / tls-native-roots to ark-grpc, so the right #[cfg] paths activate.
• Default features (tls-webpki-roots) mean the common case — cargo build with defaults — gets webpki roots applied, which is the correct fix for the reported issue.
• Not protocol-critical. This is transport-layer plumbing; no VTXO, signing, forfeit, or round-lifecycle code is touched.

Nit (non-blocking)

ark-grpc/src/client.rs — dual-feature edge case. If a consumer enables both tls-webpki-roots and tls-native-roots, both with_webpki_roots() and with_native_roots() are called sequentially. In tonic 0.14, the second call likely overrides the first (native roots win). This is harmless in practice — Cargo feature flags are additive and enabling both is unusual — but a #[cfg(all(...))] compile error or a doc comment noting the precedence would prevent future confusion. Not a blocker.

LGTM. Ship it.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ark-grpc/src/client.rs`:
- Around line 123-131: The TLS config is being applied unconditionally which can
wrap plaintext HTTP endpoints and break runtime behavior; before calling
endpoint.tls_config(tls) (where tls is a tonic::transport::ClientTlsConfig
created in the cfg block), check the endpoint URI/scheme and only invoke
Endpoint::tls_config when the scheme is https (or otherwise indicates TLS).
Locate the code that builds the endpoint (the variable endpoint) and the
ClientTlsConfig construction and add a guard that inspects endpoint.uri() or
equivalent scheme parsing so tls_config is only called for https schemes,
returning or leaving endpoint unchanged for http.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cc4c5014-527d-4619-8be5-9260f582474f

📥 Commits

Reviewing files that changed from the base of the PR and between 70eaa75 and 100c53a.

📒 Files selected for processing (1)

• ark-grpc/src/client.rs

[luckysori]

Thank you for the fix!

This is my bad. I specifically ran into this bug locally testing against mutinynet and fixed it, but forgot to merge the fix.

I will merge and deploy 0.9.3 now.