fix(config): use named constants for fallbacks, remove dead code, fix stale doc ref (#673, #674, #675, #678) - #860
Open
trakshan-mishra wants to merge 1 commit into
Conversation
… stale doc ref Bundle four config-module cleanups (good-first-issues SO4-Markets#673, SO4-Markets#674, SO4-Markets#675, SO4-Markets#678): - SO4-Markets#675: bind_addr collect_or_default fallback hardcoded "0.0.0.0:8080" instead of reusing DEFAULT_BIND_ADDR; a future edit to the constant would silently desync from the fallback with no test catching it. Replaced with DEFAULT_BIND_ADDR.parse().unwrap() and added a unit test asserting Config::from_lookup resolves to DEFAULT_BIND_ADDR when BIND_ADDR is unset. - SO4-Markets#674: the ENV_KEY constant ("PRICE_FEED_CONFIG") was declared but never read — the actual lookup at line 204 hardcoded the literal string. Replaced the literal with ENV_KEY so the constant is the single source of truth for the env-var name. - SO4-Markets#673: MAINNET_RPC_URL was defined but never referenced anywhere in the workspace. Mainnet intentionally requires explicit STELLAR_RPC_URL (enforced by config_from_lookup_requires_explicit_mainnet_rpc), so there is no fallback path to wire the constant into. Removed the dead constant to stop implying a code path that doesn't exist. - SO4-Markets#678: validate_strkey() doc comment cited issue SO4-Markets#3 for CRC16/keypair derivation validation, but SO4-Markets#3 is an unrelated closed feature ticket. Reworded to describe the limitation accurately without the stale issue pointer. cargo fmt, cargo clippy --all-targets -- -D warnings, and cargo test --workspace all pass (296+ tests, 0 failures). Closes SO4-Markets#673, SO4-Markets#674, SO4-Markets#675, SO4-Markets#678
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.
Description
Bundles four config-module cleanups, all good-first-issues in
oracle/src/config.rsandoracle/src/network_config.rs. Each is small, independent, and addresses the exact acceptance criteria in its issue.#675 —
bind_addrfallback duplicates the literal instead of reusingDEFAULT_BIND_ADDRcollect_or_default!forbind_addrused"0.0.0.0:8080".parse().unwrap()as its fallback, while every other tunable in the file reuses itsDEFAULT_*constant. A future edit toDEFAULT_BIND_ADDRwould silently desync from the fallback with no test catching it.Fix: replaced the literal with
DEFAULT_BIND_ADDR.parse().unwrap()and added a unit test (config_from_lookup_bind_addr_defaults_to_constant) assertingConfig::from_lookupresolves toDEFAULT_BIND_ADDRwhenBIND_ADDRis unset.#674 —
ENV_KEYconstant is unused; the literal"PRICE_FEED_CONFIG"is duplicated at the lookupENV_KEYwas declared as the single source of truth for the env-var name, but the actual lookup at line 204 hardcoded the literal string. If the var name were ever renamed,ENV_KEYwould silently go stale while the real lookups kept working off the duplicated literal.Fix: replaced the literal at the lookup site with
ENV_KEY. (The remainingPRICE_FEED_CONFIGreferences inshared/config/src/lib.rsare user-facing error messages and doc comments in a separate crate — not lookups — so they stay as literal strings describing the var name to users.)#673 —
MAINNET_RPC_URLis defined but never referencedMAINNET_RPC_URLhad no call site anywhere in the workspace. OnNetwork::Mainnet,config.rscallsrequired(&mut lookup, "STELLAR_RPC_URL")unconditionally — there is no fallback path. This is intentional: the testconfig_from_lookup_requires_explicit_mainnet_rpcasserts that mainnet forces explicit RPC configuration (a safety measure). Wiring the constant as a fallback would break that test and the intentional design.Fix: removed the dead constant rather than wiring it in, so the code stops implying a code path that doesn't exist.
MAINNET_PASSPHRASE(which is used) is retained.#678 —
validate_strkey()doc comment cites an unrelated issue #3The doc comment claimed CRC16/keypair-derivation validation is "wired with the keeper in #3", but #3 is an unrelated closed data-store rent estimation feature. No tracking issue exists for the CRC16/derivation gap.
Fix: reworded the doc comment to accurately describe the limitation without the stale issue pointer: full keypair verification happens in the keeper runtime, not at config load.
Type of change
Verification
The new test
config_from_lookup_bind_addr_defaults_to_constantpasses, and the existingconfig_from_lookup_requires_explicit_mainnet_rpctest still passes (confirming theMAINNET_RPC_URLremoval doesn't affect the intentional mainnet-requires-explicit-RPC behavior).Related issues
Closes #673, #674, #675, #678