Default api_url, auth_url, network, chain_id so boilerplate can omit them - #12
Conversation
These four are the same for every mainnet customer, so default them to the public Optimum endpoints and Ethereum mainnet and let the dashboard boilerplate omit them. An explicit env var or flag still wins (resolved in from_env via 'or DEFAULT', mirrored as model field defaults). api_url -> https://console.getoptimum.io auth_url -> https://auth.getoptimum.io network -> mainnet chain_id -> 0x1 Updates the README config table + quickstart and the CLI flag help, and adds tests for defaults-applied-when-unset and explicit-override. A testnet user must still set network + chain_id together (documented).
📝 WalkthroughWalkthroughAdds default values for Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
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 `@src/optimum_keysync/config.py`:
- Around line 90-93: The config fields `network` and `chain_id` in `config.py`
are currently independently overridable, which allows mixed-chain values like
`network="hoodi"` with the default `chain_id`. Update the validation logic in
the `Config` model so partial overrides are rejected and only the pair is
accepted together; keep the default behavior unchanged when neither is provided.
Use the existing `network`/`chain_id` fields and the related config-building
logic around the cited lines to enforce the coupled override rule.
🪄 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: Repository: getoptimum/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d7880e57-7bb9-4bf3-96fb-13f66a974ae9
📒 Files selected for processing (4)
README.mdsrc/optimum_keysync/cli.pysrc/optimum_keysync/config.pytests/test_config.py
| # network/chain_id default to Ethereum mainnet. Override both together for | ||
| # a testnet (e.g. network=hoodi with its chain_id). | ||
| network: Annotated[str, Field(min_length=1)] = _DEFAULT_NETWORK | ||
| chain_id: Annotated[str, Field(min_length=1)] = _DEFAULT_CHAIN_ID |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Enforce network/chain_id as a coupled override to prevent mixed-chain configs.
Line 177/Line 178 can produce network="hoodi" with chain_id="0x1" when only one is set, which silently creates an inconsistent target chain config. Please reject partial overrides and require both values together when overriding defaults.
Suggested minimal fix
def from_env(overrides: dict[str, str | Path | None] | None = None) -> KeysyncConfig:
@@
- return KeysyncConfig(
+ network_override = pick("network")
+ chain_id_override = pick("chain_id")
+ if (network_override is None) ^ (chain_id_override is None):
+ raise ValueError(
+ "KEYSYNC_NETWORK and KEYSYNC_CHAIN_ID must be set together when overriding defaults"
+ )
+
+ return KeysyncConfig(
api_url=pick("api_url") or _DEFAULT_API_URL,
auth_url=pick("auth_url") or _DEFAULT_AUTH_URL,
@@
- network=pick("network") or _DEFAULT_NETWORK,
- chain_id=pick("chain_id") or _DEFAULT_CHAIN_ID,
+ network=network_override or _DEFAULT_NETWORK,
+ chain_id=chain_id_override or _DEFAULT_CHAIN_ID,Also applies to: 173-178
🤖 Prompt for 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.
In `@src/optimum_keysync/config.py` around lines 90 - 93, The config fields
`network` and `chain_id` in `config.py` are currently independently overridable,
which allows mixed-chain values like `network="hoodi"` with the default
`chain_id`. Update the validation logic in the `Config` model so partial
overrides are rejected and only the pair is accepted together; keep the default
behavior unchanged when neither is provided. Use the existing
`network`/`chain_id` fields and the related config-building logic around the
cited lines to enforce the coupled override rule.
What
Defaults the four config values that are identical for every mainnet customer, so the dashboard's API-key boilerplate can drop them:
KEYSYNC_API_URLhttps://console.getoptimum.ioKEYSYNC_AUTH_URLhttps://auth.getoptimum.ioKEYSYNC_NETWORKmainnetKEYSYNC_CHAIN_ID0x1An explicit env var or flag still wins. Resolved in
from_envviaor <default>and mirrored as pydantic model field defaults, so both the env path and direct construction get them.After this, the minimal config is just
KEYSYNC_API_KEY,KEYSYNC_OWNER_NAME, and one validator source (+KEYSYNC_BEACON_URLwhen using pubkeys/index input).Notes
networkandchain_iddefault to mainnet, so a testnet user must set both together (e.g.KEYSYNC_NETWORK=hoodiwith its chain id). Documented in the README and flag help. A wrong/forgotten value here scopes reads/writes to the wrong network rather than failing loudly, which is the one tradeoff of defaulting these.KEYSYNC_API_KEY(required secret) orKEYSYNC_OWNER_NAME(handled separately).Tests
test_url_network_chain_defaults_applied_when_unset(clears env, omits the four, asserts the defaults)test_explicit_url_network_chain_override_defaultspytest: 79 passed;ruffclean;mypyclean.Written with Claude Code
Summary by CodeRabbit