Skip to content

Drop owner_name (org attribution comes from the API key) - #13

Merged
CryptoFewka merged 1 commit into
mainfrom
chore/drop-owner-name
Jun 25, 2026
Merged

Drop owner_name (org attribution comes from the API key)#13
CryptoFewka merged 1 commit into
mainfrom
chore/drop-owner-name

Conversation

@CryptoFewka

@CryptoFewka CryptoFewka commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Draft — do not merge yet. Sequencing notes at the bottom.

What

Removes owner_name / KEYSYNC_OWNER_NAME from keysync entirely. It was a client-supplied label on the shared eth_validator_keys registry, but it never determined which org a validator lands in: that is created_by_operator_id + the assignment, both derived from the operator API key. The console API now defaults the registry label from the caller's operator_name server-side (getoptimum/optimum-cross-functional-dashboard#223), so the client doesn't need to send it.

Changes

  • config: drop the owner_name field and its from_env wiring.
  • cli: drop --owner-name / KEYSYNC_OWNER_NAME and the resolve() argument.
  • api_client: drop owner_name from ValidatorKeyRegistration and the /validator-keys/batch payload.
  • desired_state: drop owner_name from DesiredValidator, resolve(), and _read_indices (the indices JSON no longer needs it).
  • reconcile: drop owner_name from the registration it builds.
  • docs/examples/tests: README config table + quickstart, k8s CronJob comment, examples/indices.json, the test fixture, and all tests updated; removed the obsolete owner_name length-cap test.

After this, the required config is just KEYSYNC_API_KEY + a validator source (+ KEYSYNC_BEACON_URL for pubkeys/index input). Everything else defaults.

  • pytest: 76 passed
  • ruff / mypy: clean

⚠️ Sequencing (why this is a draft)

  1. Conflicts with open PRs. Branched off current main, so it overlaps with Harden API response handling, config validation, and active-aware reconcile #11 (api_client/reconcile/desired_state) and Default api_url, auth_url, network, chain_id so boilerplate can omit them #12 (config). Rebase after those merge.
  2. Release gate. Do not release a keysync version without owner_name until dashboard #223 (server-side default) is deployed to prod — otherwise the batch register still 400s on a missing owner_name.
  3. After this ships, a follow-up drops the KEYSYNC_OWNER_NAME line from the dashboard's API-key boilerplate.

Written with Claude Code

Summary by CodeRabbit

  • Documentation

    • Updated setup and configuration docs to remove the owner-name setting and reflect the current validator record format.
    • Revised the Kubernetes example comments to match the supported configuration options.
  • Bug Fixes

    • Simplified validator registration and planning so records now use only validator index, chain ID, and validator key.
    • Removed the owner-name CLI/config option, keeping configuration and generated outputs consistent across commands.

@CryptoFewka
CryptoFewka force-pushed the chore/drop-owner-name branch from d3c43ba to 819fd9b Compare June 25, 2026 16:07
owner_name was a client-supplied label on the shared eth_validator_keys
registry, but it never drove org membership: that is created_by_operator_id
plus the assignment, both derived from the operator API key. Remove it end to
end so customers no longer set KEYSYNC_OWNER_NAME; the console API now defaults
the registry label from the caller's operator_name.

- config: drop owner_name field + from_env wiring
- cli: drop --owner-name / KEYSYNC_OWNER_NAME and the resolve() arg
- api_client: drop owner_name from ValidatorKeyRegistration + batch payload
- desired_state: drop owner_name from DesiredValidator, resolve(), _read_indices
- reconcile: drop owner_name from the registration it builds
- README, k8s example, indices/fixtures, tests updated

76 passed, ruff and mypy clean.
@CryptoFewka
CryptoFewka force-pushed the chore/drop-owner-name branch from 819fd9b to ef04a63 Compare June 25, 2026 16:12
@CryptoFewka
CryptoFewka marked this pull request as ready for review June 25, 2026 16:44
@CryptoFewka
CryptoFewka merged commit ae5d117 into main Jun 25, 2026
2 of 3 checks passed
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: getoptimum/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 82df7420-a721-44b8-9143-2331534a8e7f

📥 Commits

Reviewing files that changed from the base of the PR and between 6ebb4db and ef04a63.

⛔ Files ignored due to path filters (2)
  • examples/indices.json is excluded by none and included by none
  • tests/fixtures/validators.json is excluded by none and included by none
📒 Files selected for processing (13)
  • README.md
  • examples/kubernetes/cronjob.yaml
  • src/optimum_keysync/api_client.py
  • src/optimum_keysync/cli.py
  • src/optimum_keysync/config.py
  • src/optimum_keysync/desired_state.py
  • src/optimum_keysync/reconcile.py
  • tests/test_api_client.py
  • tests/test_cli_apply.py
  • tests/test_cli_lazy_me.py
  • tests/test_config.py
  • tests/test_desired_state.py
  • tests/test_reconcile.py

📝 Walkthrough

Walkthrough

The PR removes owner_name from the config, CLI, desired-state model, registration payloads, and related docs/tests. KEYSYNC_OWNER_NAME is no longer documented or read from config. Desired-state records, API registration requests, and reconciliation inputs now use only validator_index, chain_id, and validator_key.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant