feat(config): Add Environment Variable Support for RPC and Explorer URLs in configuration #517
Conversation
WalkthroughAdds a new example demonstrating environment-driven RPC URL configuration, including docs, config files, and docker-compose. Introduces StringOrEnvValue to support env-variable URL resolution, updates network config types and merging, and modifies EVM/Solana/Stellar model construction to resolve URLs. Broad test updates align with the new wrapper. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant ENV as Environment
participant CFG as Config Loader
participant SOE as StringOrEnvValue
participant NCC as NetworkConfigCommon
participant MODELS as Network Models<br/>(EVM/Solana/Stellar)
participant RUNTIME as Relayer/Tx Logic
CFG->>NCC: Parse config (rpc/explorer URLs as SOE[])
MODELS->>NCC: resolve_rpc_urls()/resolve_explorer_urls()
NCC->>SOE: resolve_all(values)
loop For each value
SOE->>SOE: is_plain / is_env
alt Env reference
SOE->>ENV: read VAR
ENV-->>SOE: value or missing
end
end
SOE-->>NCC: Vec<String> or error
NCC-->>MODELS: Resolved URLs or InvalidFormat
MODELS-->>RUNTIME: Constructed networks with resolved URLs
Note over MODELS,RUNTIME: Errors mapped to RepositoryError::InvalidData
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
All contributors have signed the CLA ✍️ ✅ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (25)
examples/env-rpc-urls/.env.example (1)
7-11: Reorder Sepolia vars to satisfy dotenv-linter (alphabetical).Current order triggers UnorderedKey warnings. Reorder EXPLORER_API, FALBACK, PRIMARY.
# Sepolia Testnet -SEPOLIA_RPC_URL_PRIMARY=https://sepolia.infura.io/v3/YOUR_INFURA_KEY -SEPOLIA_RPC_URL_FALLBACK=https://sepolia.alchemy.com/v2/YOUR_ALCHEMY_KEY -SEPOLIA_EXPLORER_API=https://api-sepolia.etherscan.io/api +SEPOLIA_EXPLORER_API=https://api-sepolia.etherscan.io/api +SEPOLIA_RPC_URL_FALLBACK=https://sepolia.alchemy.com/v2/YOUR_ALCHEMY_KEY +SEPOLIA_RPC_URL_PRIMARY=https://sepolia.infura.io/v3/YOUR_INFURA_KEYexamples/env-rpc-urls/config/config.json (1)
41-50: Avoid empty webhook URL in example; provide a usable placeholder.An empty string will likely fail at startup or during tests. Use a placeholder or demonstrate env-based configuration.
- "url": "", + "url": "https://webhook.site/YOUR_UNIQUE_URL",Alternatively:
- "url": "", + "url": { "type": "env", "value": "WEBHOOK_URL" },examples/env-rpc-urls/docker-compose.yaml (1)
23-31: Pin Redis image and add healthcheck for readiness.Using latest is non-deterministic; add a healthcheck and a restart policy for dev stability.
- redis: - image: redis:latest + redis: + image: redis:7.2-alpine ports: - 6379:6379 networks: - relayer-network - command: redis-server --appendonly yes + command: redis-server --appendonly yes + healthcheck: + test: ["CMD", "redis-cli", "ping"] + interval: 5s + timeout: 3s + retries: 10 volumes: - redis-data:/dataOptionally add restart policies:
relayer: build: @@ volumes: - ./config:/app/config:ro + restart: unless-stopped @@ redis: @@ - volumes: + restart: unless-stopped + volumes: - redis-data:/dataexamples/env-rpc-urls/config/networks/custom.json (1)
22-28: Demonstrate fallback RPC via env var to match .env.example.You define SEPOLIA_RPC_URL_FALLBACK in .env.example; include it here for resilience.
"rpc_urls": [ - {"type": "env", "value": "SEPOLIA_RPC_URL_PRIMARY"} + {"type": "env", "value": "SEPOLIA_RPC_URL_PRIMARY"}, + {"type": "env", "value": "SEPOLIA_RPC_URL_FALLBACK"} ],examples/env-rpc-urls/README.md (3)
196-201: Add language to fenced error blocks (markdownlint).Specify a language for fenced code (use text).
-``` +```text Failed to resolve RPC URL: Environment variable 'ETH_MAINNET_RPC_URL' not found--- `204-209`: **Add language to fenced error blocks (markdownlint).** Specify a language for fenced code (use text). ```diff -``` +```text Invalid RPC URL: <your-url>--- `120-123`: **Compose v2 naming (optional): prefer `docker compose up`.** Modern Docker uses `docker compose` (space), though `docker-compose` still works if installed. </blockquote></details> <details> <summary>src/config/config_file/network/common.rs (3)</summary><blockquote> `11-11`: **Doc nit: include Explorer URLs too** Text says only “RPC URLs”; explorer URLs also support env refs. Update for accuracy. --- `40-51`: **Return typed errors from resolvers** Prefer returning ConfigFileError (or StringOrEnvError) instead of String for resolve_rpc_urls/resolve_explorer_urls to keep error typing consistent with the config layer. Also applies to: 52-62 --- `82-110`: **Use url::Url instead of reqwest::Url for parsing** Config validation shouldn’t depend on reqwest. url::Url is lighter and purpose-built for parsing. Apply this diff: ```diff - for url in resolved { - reqwest::Url::parse(&url).map_err(|_| { + for url in resolved { + url::Url::parse(&url).map_err(|_| { ConfigFileError::InvalidFormat(format!("Invalid RPC URL: {}", url)) })?; }And for Explorer URLs:
- for url in resolved { - reqwest::Url::parse(&url).map_err(|_| { + for url in resolved { + url::Url::parse(&url).map_err(|_| { ConfigFileError::InvalidFormat(format!("Invalid Explorer URL: {}", url)) })?; }Add the import:
+use url;Based on learnings
src/config/config_file/network/string_or_env.rs (3)
20-24: Treat empty environment variables as errorsCurrently, VAR="" resolves to Ok(""), which is often misconfiguration. Recommend erroring on empty as well.
Apply this diff:
#[derive(Error, Debug)] pub enum StringOrEnvError { #[error("Environment variable '{0}' not found")] MissingEnvVar(String), + #[error("Environment variable '{0}' is empty")] + EmptyEnvVar(String), }And in resolve():
- Self::Env(env_ref) => { - match env_ref.type_field { - ValueRefType::Env => std::env::var(&env_ref.value) - .map_err(|_| StringOrEnvError::MissingEnvVar(env_ref.value.clone())), // Future: Add other types here - // ValueRefType::Vault => resolve_from_vault(&env_ref.value), - // ValueRefType::AwsSecret => resolve_from_aws(&env_ref.value), - } - } + Self::Env(env_ref) => match env_ref.type_field { + ValueRefType::Env => { + match std::env::var(&env_ref.value) { + Ok(v) if v.is_empty() => Err(StringOrEnvError::EmptyEnvVar(env_ref.value.clone())), + Ok(v) => Ok(v), + Err(_) => Err(StringOrEnvError::MissingEnvVar(env_ref.value.clone())), + } + } + }Also applies to: 66-79
92-99: Reduce accidental secret leakage in DisplayPlain values may include API keys (query strings). Consider a redacted Display or a separate redact_for_logs() that masks query params.
52-65: Ergonomics: add From implsAdd From<&str>/From to construct Plain more idiomatically; optional but improves call sites.
Example:
impl From<&str> for StringOrEnvValue { fn from(s: &str) -> Self { Self::plain(s) } } impl From<String> for StringOrEnvValue { fn from(s: String) -> Self { Self::Plain(s) } }src/config/config_file/mod.rs (1)
193-194: Re-export convenience (optional)Consider re-exporting StringOrEnvValue (and ValueRefType) at this module root (pub use network::StringOrEnvValue) so test and downstream code can import crate::config::StringOrEnvValue directly.
src/api/controllers/relayer.rs (1)
852-864: Optional: use a scoped env guard in testsTo ensure env vars are always cleaned up even on panic, consider a small RAII guard that sets vars in new() and removes in Drop, instead of manual setup/cleanup calls.
src/config/config_file/network/test_utils.rs (1)
21-25: Good coverage of plain-string paths; consider adding env-ref JSON coverageThe helpers now build rpc/explorer URLs via StringOrEnvValue::plain(...). To exercise the new env-ref format end-to-end, add a JSON fixture using the {"type":"env","value":"..."} shape and a test that sets the env var and resolves it.
I can draft a small test JSON and a test case that sets/unsets the env to verify resolution and error on missing env.
Also applies to: 106-113, 126-133, 146-153, 168-174
src/domain/relayer/solana/solana_relayer.rs (1)
871-873: Good switch to StringOrEnvValue; add ENV-resolution test coverage.Please add a test that uses the env-object form ({"type":"env","value":"..."}), exercising resolution and the missing-variable error path. This likely belongs in the config loader/common tests but ensures end-to-end parity.
src/models/network/repository.rs (1)
162-163: Tests aligned with StringOrEnvValue — nice. Consider adding serde round‑trip checks for URLs.Current round-trip test only checks id/name/type. Add assertions that rpc_urls/explorer_urls survive serialize/deserialize with the wrapper to prevent regressions.
Also applies to: 171-175, 193-196, 208-210, 346-347, 466-466
src/utils/mocks.rs (1)
11-13: Mocks updated to wrapper — looks good.Optional: use a Solana-typical URL/port (e.g., 8899 or devnet) in create_mock_solana_network for clarity.
Also applies to: 102-103, 126-127
src/config/config_file/network/inheritance.rs (1)
119-120: Inheritance tests updated — consider adding ENV-based cases.Add a case where parent (or child) supplies ENV refs and the other supplies Plain, to verify override behavior and that resolution later surfaces missing-variable errors correctly.
Also applies to: 536-542, 559-569, 673-675, 700-706, 723-727, 729-733
src/config/config_file/network/mod.rs (1)
553-554: Validation test update looks good; add parse test for ENV-object form.Include a JSON deserialization test that uses {"type":"env","value":"ENV_NAME"} in rpc_urls/explorer_urls to assert backward-compatible parsing (resolution can be tested elsewhere).
src/config/config_file/network/evm.rs (1)
168-169: Tests correctly migrated to StringOrEnvValue. Add a validation case for ENV-only rpc_urls.Please add a test where rpc_urls contains only ENV refs to confirm common.validate() behavior (accept or reject) matches intent and docs.
Also applies to: 240-241, 301-307, 326-332, 354-357, 360-363, 393-399, 423-426, 429-432, 456-462, 481-487, 507-510, 513-516, 605-609, 763-767
src/models/network/evm/network.rs (1)
79-89: EVM rpc_urls default-to-empty can mask misconfig; consider failing when absent/empty (align with Solana/Stellar).Currently unwrap_or_default() returns an empty list when none are provided. That diverges from Solana/Stellar (which error) and can hide missing RPCs until later. Recommend erroring if missing or empty for consistency and earlier feedback.
Proposed change:
- let rpc_urls = common - .resolve_rpc_urls() - .map_err(|e| { - RepositoryError::InvalidData(format!( - "Failed to resolve RPC URLs for network '{}': {}", - network_repo.name, e - )) - })? - .unwrap_or_default(); + let rpc_urls = common + .resolve_rpc_urls() + .map_err(|e| { + RepositoryError::InvalidData(format!( + "Failed to resolve RPC URLs for network '{}': {}", + network_repo.name, e + )) + })? + .ok_or_else(|| { + RepositoryError::InvalidData(format!( + "EVM network '{}' has no rpc_urls", + network_repo.name + )) + })?; + if rpc_urls.is_empty() { + return Err(RepositoryError::InvalidData(format!( + "EVM network '{}' resolved an empty rpc_urls list", + network_repo.name + ))); + }If the current behavior is intentional for EVM, please confirm and add a brief comment explaining the divergence.
src/models/network/stellar/network.rs (1)
40-55: Validate non-empty rpc_urls after resolution.You error on None, but allow Some(empty). That likely indicates misconfig and will degrade UX later. Recommend failing when the list is empty.
Apply after rpc_urls resolution:
let rpc_urls = common .resolve_rpc_urls() .map_err(|e| { RepositoryError::InvalidData(format!( "Failed to resolve RPC URLs for network '{}': {}", network_repo.name, e )) })? .ok_or_else(|| { RepositoryError::InvalidData(format!( "Stellar network '{}' has no rpc_urls", network_repo.name )) })?; + if rpc_urls.is_empty() { + return Err(RepositoryError::InvalidData(format!( + "Stellar network '{}' resolved an empty rpc_urls list", + network_repo.name + ))); + }src/models/network/solana/network.rs (1)
35-50: Enforce non-empty rpc_urls after resolution.You error on missing rpc_urls but allow an empty vector. Recommend treating empty as invalid to fail fast.
let rpc_urls = common .resolve_rpc_urls() .map_err(|e| { RepositoryError::InvalidData(format!( "Failed to resolve RPC URLs for network '{}': {}", network_repo.name, e )) })? .ok_or_else(|| { RepositoryError::InvalidData(format!( "Solana network '{}' has no rpc_urls", network_repo.name )) })?; + if rpc_urls.is_empty() { + return Err(RepositoryError::InvalidData(format!( + "Solana network '{}' resolved an empty rpc_urls list", + network_repo.name + ))); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
examples/env-rpc-urls/.env.example(1 hunks)examples/env-rpc-urls/README.md(1 hunks)examples/env-rpc-urls/config/config.json(1 hunks)examples/env-rpc-urls/config/networks/custom.json(1 hunks)examples/env-rpc-urls/docker-compose.yaml(1 hunks)src/api/controllers/relayer.rs(3 hunks)src/api/routes/relayer.rs(2 hunks)src/config/config_file/mod.rs(11 hunks)src/config/config_file/network/common.rs(21 hunks)src/config/config_file/network/evm.rs(13 hunks)src/config/config_file/network/inheritance.rs(6 hunks)src/config/config_file/network/mod.rs(3 hunks)src/config/config_file/network/string_or_env.rs(1 hunks)src/config/config_file/network/test_utils.rs(5 hunks)src/domain/relayer/evm/evm_relayer.rs(2 hunks)src/domain/relayer/solana/solana_relayer.rs(2 hunks)src/domain/relayer/stellar/stellar_relayer.rs(3 hunks)src/domain/transaction/evm/evm_transaction.rs(3 hunks)src/domain/transaction/evm/status.rs(4 hunks)src/models/network/evm/network.rs(3 hunks)src/models/network/repository.rs(5 hunks)src/models/network/solana/network.rs(1 hunks)src/models/network/stellar/network.rs(2 hunks)src/models/transaction/repository.rs(5 hunks)src/repositories/network/network_in_memory.rs(2 hunks)src/repositories/network/network_redis.rs(3 hunks)src/utils/mocks.rs(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (20)
src/config/config_file/network/mod.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/api/routes/relayer.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/models/network/repository.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/domain/transaction/evm/evm_transaction.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/config/config_file/network/evm.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/domain/relayer/evm/evm_relayer.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/repositories/network/network_redis.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/repositories/network/network_in_memory.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/utils/mocks.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/models/network/evm/network.rs (3)
src/models/network/repository.rs (3)
common(24-30)common(131-133)config(136-138)src/models/network/solana/network.rs (1)
explorer_urls(95-97)src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/config/config_file/network/test_utils.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/config/config_file/network/inheritance.rs (4)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)src/models/network/evm/network.rs (1)
explorer_urls(169-171)src/models/network/solana/network.rs (1)
explorer_urls(95-97)src/models/network/stellar/network.rs (1)
explorer_urls(114-116)
src/config/config_file/mod.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/models/network/stellar/network.rs (3)
src/models/network/repository.rs (2)
common(24-30)common(131-133)src/models/network/evm/network.rs (1)
explorer_urls(169-171)src/models/network/solana/network.rs (1)
explorer_urls(95-97)
src/api/controllers/relayer.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/domain/relayer/solana/solana_relayer.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/config/config_file/network/common.rs (1)
src/config/config_file/network/string_or_env.rs (3)
merge_optional_string_or_env_vecs(128-141)resolve_all(123-125)plain(54-56)
src/domain/relayer/stellar/stellar_relayer.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/domain/transaction/evm/status.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
src/models/transaction/repository.rs (1)
src/config/config_file/network/string_or_env.rs (1)
plain(54-56)
🪛 dotenv-linter (3.3.0)
examples/env-rpc-urls/.env.example
[warning] 9-9: [UnorderedKey] The SEPOLIA_RPC_URL_FALLBACK key should go before the SEPOLIA_RPC_URL_PRIMARY key
(UnorderedKey)
[warning] 10-10: [UnorderedKey] The SEPOLIA_EXPLORER_API key should go before the SEPOLIA_RPC_URL_FALLBACK key
(UnorderedKey)
🪛 Gitleaks (8.28.0)
examples/env-rpc-urls/README.md
[high] 133-135: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🪛 markdownlint-cli2 (0.18.1)
examples/env-rpc-urls/README.md
198-198: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
205-205: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Redirect rules - openzeppelin-relayer
- GitHub Check: Header rules - openzeppelin-relayer
- GitHub Check: Pages changed - openzeppelin-relayer
- GitHub Check: semgrep/ci
🔇 Additional comments (18)
src/config/config_file/network/common.rs (1)
123-127: Merge semantics LGTMChild rpc_urls/explorer_urls fully override parent as intended.
src/config/config_file/network/string_or_env.rs (3)
101-120: Deserialize approach LGTMManual Deserialize keeps backward compatibility with strings; #[serde(untagged)] on the enum pairs well with Serialize. Looks good.
122-126: resolve_all correctnessCollects with short-circuiting on first error. Simple and correct.
127-141: Override merge semantics for URLs are appropriateChild wins entirely; matches stated inheritance rules.
src/domain/transaction/evm/evm_transaction.rs (1)
777-779: Tests updated correctly to new URL wrapperImport and usage of StringOrEnvValue::plain(...) align with new config types. No issues.
Also applies to: 1214-1220, 1401-1403
src/api/routes/relayer.rs (1)
222-223: Route tests aligned with env-aware URLsImport and wrapper usage look correct and keep fixtures in sync with config changes.
Also applies to: 269-270
src/config/config_file/mod.rs (1)
41-45: Public API change: type of rpc_urls/explorer_urlsNetworkConfigCommon now exposes Vec. If this crate is consumed externally, this is a compile-time breaking change despite JSON backward compatibility. Please confirm it’s acceptable (or consider a type alias/newtype to ease migration).
src/api/controllers/relayer.rs (1)
835-835: Tests updated to use StringOrEnvValue look goodImport and usage via StringOrEnvValue::plain(...) align with the new config type; this keeps tests consistent with env-aware URLs.
Also applies to: 900-901, 923-925
src/repositories/network/network_redis.rs (2)
651-651: LGTM: test fixtures migrated to StringOrEnvValueThe move to StringOrEnvValue::plain(...) in test network builders is consistent and non-invasive.
Also applies to: 663-664, 951-952
236-279: Verify serialization format stability for new URL wrapperSince NetworkRepoModel is serialized to Redis, please verify that StringOrEnvValue serialization keeps backward compatibility (plain strings remain plain; env refs as objects) and that deserialization accepts both formats as intended.
Based on learnings
src/domain/transaction/evm/status.rs (1)
384-391: LGTM: tests now use StringOrEnvValue for rpc/explorer URLsImports and wrappers are correct; domain logic tests remain unchanged.
Also applies to: 456-460, 484-488, 863-867
src/repositories/network/network_in_memory.rs (1)
172-175: LGTM: in-memory repo tests updated to StringOrEnvValueUsing StringOrEnvValue::plain(...) in test builders aligns with the new config schema.
Also applies to: 183-184
src/domain/relayer/evm/evm_relayer.rs (1)
616-617: LGTM: repo model builder uses StringOrEnvValueThe test network config now wraps URLs correctly; domain tests remain unaffected.
Also applies to: 662-665
src/domain/relayer/solana/solana_relayer.rs (1)
780-781: LGTM on the import.Importing StringOrEnvValue from config::network matches the new re-export and keeps tests clean.
src/config/config_file/network/mod.rs (1)
28-29: Publicly exposing string_or_env is the right call.The re-export keeps import sites tidy and consistent.
Also applies to: 39-40
src/models/network/evm/network.rs (1)
185-186: LGTM: tests updated to use StringOrEnvValue wrapper.Import and usage of StringOrEnvValue::plain(...) in tests are correct and keep configs explicit.
Optional: add one test path that uses the env variant (type: env) to exercise failure messaging when the env var is missing.
Also applies to: 293-304
src/models/transaction/repository.rs (1)
1061-1063: LGTM: config tests migrated to StringOrEnvValue.The import and usage of StringOrEnvValue::plain(...) across EVM/Solana/Stellar tests correctly match the new schema.
Optional: add a test that supplies an env reference and asserts TransactionRepoModel::try_from errors when the env var is unset, to cover the resolution path end-to-end.
Also applies to: 1671-1673, 1749-1754, 1830-1831, 2088-2089
src/domain/relayer/stellar/stellar_relayer.rs (1)
478-479: LGTM: tests now wrap RPC URLs with StringOrEnvValue.The migration to StringOrEnvValue::plain(...) is correct and keeps the test setup aligned with the new config format.
Optional: add a test that uses an env ref and verifies StellarRelayer::new fails with a clear error when the env var is missing.
Also applies to: 545-547, 1014-1015
|
I confirm that I have read and hereby agree to the OpenZeppelin Contributor License Agreement |
recheck |
0xpetersatoshi
changed the title
zeljkoX
left a comment
zeljkoX
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
This PR looks great and adds important logic.
I’ve added a few comments in the code. I’d like to discuss whether we can use an existing similar model instead of introducing a new one — or perhaps just wrap the existing one.
Thanks for adding the example as well!
In addition, we’ll need to update the documentation here
.
We should also support this feature at the relayer level, where the custom_rpc_urls field allows using specific RPC URLs for particular relayers, to keep things consistent.
| @@ -0,0 +1,289 @@ | |||
| //! String or Environment Variable Value Module | |||
There was a problem hiding this comment.
I wonder if you’ve checked src/models/plain_or_env_value.rs?
It’s almost the same as this one, but includes improvements for runtime secret handling using Zeroize and SecretsVec.
Perhaps backward compatibility was the reason for introducing a new model.
The existing model is already used for similar logic where secure handling is required and where configuration is loaded.
If feasible, it would be best to use the original one so that once we add support for secret providers (like Vault and others), everything remains centralized in a single place.
There was a problem hiding this comment.
Yeah, backwards compatibility was the main concern but I think long term having all the logic centralized in src/models/plain_or_env_value.rs is the better move. I can refactor to use that existing model instead.
There was a problem hiding this comment.
I see. Maybe creating a new model that wraps PlainOrEnvValue and adds support for strings would work. This approach might also help prevent potential changes to other parts of the codebase.
2 participants
Summary
This PR adds support for referencing environment variables in network configuration files for RPC and Explorer URLs, addressing the security concern of storing API keys in plaintext configuration files.
Problem
Previously, RPC URLs could only be provided as plaintext strings in the configuration files. This posed a security risk when URLs contained sensitive API keys (e.g.,
https://eth-mainnet.alchemyapi.io/v2/YOUR-API-KEY), as these would be exposed in the repository or configuration files.Solution
Implemented a backward-compatible configuration format that allows URLs to be specified either as:
"https://eth.drpc.org"{"type": "env", "value": "ETH_RPC_URL"}Key Design Decisions
Backward Compatibility: Existing configurations continue to work without modification. The system automatically detects whether a value is a plain string or an environment reference during deserialization.
Type-Safe Implementation: Used an enum
ValueRefTypeinstead of string literals for the type field, ensuring compile-time type safety and making the code more maintainable.Extensible Architecture: The design allows for future expansion to other secret sources (HashiCorp Vault, AWS Secrets Manager, etc.) by adding new variants to the
ValueRefTypeenum.Runtime Resolution: Environment variables are resolved at runtime when the network configuration is loaded, with clear error messages if variables are not found.
Changes
Core Implementation
src/config/config_file/network/string_or_env.rsmodule with:StringOrEnvValueenum for dual string/env supportValueRefTypeenum for type-safe value referencesConfiguration Updates
NetworkConfigCommonto useOption<Vec<StringOrEnvValue>>for URLsresolve_rpc_urls()andresolve_explorer_urls()helper methodsNetwork Model Updates
Testing
Documentation
examples/env-rpc-urls/showing:.envfileUsage Example
{ "networks": { "ethereum-mainnet": { "chain_id": 1, "rpc_urls": [ { "type": "env", "value": "ETH_MAINNET_RPC_URL" }, "https://eth.drpc.org" ], "explorer_urls": [{ "type": "env", "value": "ETH_EXPLORER_API_URL" }] } } }With environment variables:
ETH_MAINNET_RPC_URL=https://eth-mainnet.alchemyapi.io/v2/YOUR-API-KEY ETH_EXPLORER_API_URL=https://api.etherscan.io/api?apikey=YOUR-API-KEYTesting Process
Unit Tests
src/config/config_file/network/string_or_env.rscovering:resolve_all()helperIntegration Tests
StringOrEnvValuetypeManual Testing
examples/env-rpc-urls/:Validation Steps
cargo test- All tests passcargo clippy- No warningscargo fmt --check- Code properly formattedBreaking Changes
None. This change is fully backward compatible.
Checklist