Support multiple validator operator wallets#902
Conversation
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR introduces a ChangesOperator Wallet Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant View as link_by_operator view
participant DB as Database
Client->>View: POST operator_address
View->>DB: select_for_update ValidatorOperatorWallet by address
View->>DB: lock matching ValidatorWallet rows by operator_address
View->>DB: check wallets linked to different validator
alt conflict with different validator
View-->>Client: 409 conflict
else no conflict
View->>DB: create/reuse ValidatorOperatorWallet
View->>DB: link matching ValidatorWallet rows to validator
View-->>Client: wallets_linked, operator_wallet
end
Poem: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@backend/validators/admin.py`:
- Around line 22-37: The `wallet_count` logic is duplicated between
`ValidatorOperatorWalletInline` and `ValidatorOperatorWalletAdmin`, so
consolidate it into one shared implementation to prevent future drift. Add a
shared helper on `ValidatorOperatorWallet` (for example a
`matching_wallet_count()` method) or a module-level function that performs the
`ValidatorWallet.objects.filter(operator_address__iexact=...).count()` lookup,
then have both admin `wallet_count` methods delegate to that shared symbol while
preserving the `'-'` fallback for unsaved objects.
- Around line 103-116: `ValidatorOperatorWalletAdmin.wallet_count` is causing an
N+1 query in the changelist by counting `ValidatorWallet` per row; move this
computation into `ValidatorOperatorWalletAdmin.get_queryset` by annotating the
count on the base queryset so `list_display` can read it without extra queries.
Use the admin class and `wallet_count` method as the lookup points, and keep the
address matching consistent with the normalization performed in
`ValidatorOperatorWallet.save()` before removing `__iexact`.
In `@backend/validators/genlayer_validators_service.py`:
- Around line 584-586: The sync logic in genlayer_validators_service should not
blindly overwrite wallet.operator with None when no matching
ValidatorOperatorWallet claim or User exists. Update the operator reconciliation
around the desired_operator assignment and the wallet.operator update so
existing non-claim-based links are preserved unless you explicitly intend to
clear them, and verify that any legitimate operator linkage handled by this sync
path is always backed by a ValidatorOperatorWallet record.
- Around line 571-582: The user lookup in the operator-wallet linking logic can
fail on case-insensitive duplicates because
User.objects.get(address__iexact=wallet.operator_address) may raise
MultipleObjectsReturned. Update the lookup in this block to use a safer
case-insensitive fetch such as filter(...).first(), or otherwise handle
duplicate mixed-case rows explicitly before using user.validator and
ValidatorOperatorWallet.get_or_create.
In `@backend/validators/serializers.py`:
- Around line 81-91: The computed fields in the serializer are re-running
`_wallets(obj)` for `get_wallet_count`, `get_active_wallet_count`, and
`get_networks`, causing multiple queries per object. Update the serializer
methods in `backend/validators/serializers.py` so
`ValidatorWallet.objects.filter(operator_address__iexact=obj.address)` is
evaluated once per object (for example by caching the queryset or fetching the
needed data in one pass) and then derive the count, active count, and unique
networks from that single result inside `get_wallet_count`,
`get_active_wallet_count`, and `get_networks`.
In `@backend/validators/views.py`:
- Around line 242-277: The operator wallet claim flow in the validation view has
a race condition for first-time claims because `existing_claim` can be `None` in
concurrent requests and both can reach
`ValidatorOperatorWallet.objects.create(...)`. Update this branch to use a
conflict-safe pattern in the same validator claim logic: either `get_or_create`
or a `try/except IntegrityError` around
`ValidatorOperatorWallet.objects.create`, then re-fetch the existing row and
continue. Keep the fix localized around `existing_claim`, `wallets`, and
`operator_wallet` in the validator operator claim path.
- Around line 233-278: The first-time claim path in the operator wallet flow can
still race when ValidatorOperatorWallet.objects.create is called after the
existing_claim check, causing an uncaught IntegrityError and a 500. Update the
claim logic in backend/validators/views.py to catch that create conflict inside
the transaction.atomic block, around the ValidatorOperatorWallet.objects.create
call, and return the same 409 conflict response used for the existing_claim
path. Use the existing claim-handling logic in the validator/operator wallet
view to keep the response consistent.
In `@frontend/src/routes/ProfileEdit.svelte`:
- Around line 839-847: The operator address input in ProfileEdit.svelte lacks an
accessible name because it only uses a placeholder. Add an associated label for
operatorAddress, such as a visible or visually hidden <label
for="operatorAddress">, or provide an equivalent aria-label directly on the
input so assistive technologies can identify it.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 4d9a6144-967d-4f5e-beb0-286690b4a6d7
📒 Files selected for processing (8)
backend/validators/admin.pybackend/validators/genlayer_validators_service.pybackend/validators/migrations/0016_validatoroperatorwallet.pybackend/validators/models.pybackend/validators/serializers.pybackend/validators/tests/test_api.pybackend/validators/views.pyfrontend/src/routes/ProfileEdit.svelte
Summary
Validation
backend/env/bin/python backend/manage.py test validatorsbackend/env/bin/python backend/manage.py test validators.tests.test_api ethereum_auth.testsbackend/env/bin/python backend/manage.py makemigrations validators ethereum_auth --check --dry-rungit diff --checknode --check frontend/src/lib/api.jsfrontend/src/routes/ProfileEdit.svelteNote: full Vite build was not run because the local environment has Node 22.10.0 while Vite requires 20.19+ or 22.12+, and the Rolldown native optional binding is missing.
Summary by CodeRabbit
New Features
Bug Fixes