fix(api): complete managed onboarding without operator eERC registration - #165
fix(api): complete managed onboarding without operator eERC registration#165hitakshiA wants to merge 1 commit into
Conversation
Console onboarding hung forever at awaiting_registration on BenzoNet L1: it polled isUserRegistered(operator) but nothing ever registers the operator EOA. In managed custody the operator is an admin identity that never holds an encrypted balance (the managed treasury does, and it is eERC-registered separately during treasury provisioning), the backend has no operator key, and the console has no client-side prover, so the poll could never resolve. Gate the operator-registration poll behind a new ONBOARDING_REQUIRE_OPERATOR_REGISTRATION flag (default false = managed). When false, onboarding completes once KYC + allowlist + gas land, and any row already parked in awaiting_registration is completed rather than polled. Set true only for a self-custody deployment where operators register themselves. Verified: fresh-org onboarding on console.benzo.space now advances past KYB; new managed-path test asserts complete-after-gas with zero registration polls; existing poll-path tests pin the flag true.
📝 WalkthroughWalkthroughThe API adds a configuration flag controlling whether operator registration is required. When disabled, managed onboarding completes after gas dripping without polling for registration. Tests cover both enabled defaults and the disabled completion path. ChangesOnboarding registration flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant advanceOnboarding
participant OnboardingDB
participant RegistrationPoller
Client->>advanceOnboarding: Start onboarding
advanceOnboarding->>OnboardingDB: Read onboarding state
alt registration requirement disabled
advanceOnboarding->>OnboardingDB: Mark registration complete
OnboardingDB-->>Client: Status complete
else registration requirement enabled
advanceOnboarding->>RegistrationPoller: Poll operator registration
RegistrationPoller-->>OnboardingDB: Registration result
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
services/api/src/onboarding/service.ts (1)
519-530: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
markOnboardingRegisteredinpollRegistrationto remove the duplicate update.
pollRegistration'sregisteredbranch (further down, lines 541-547) sets the exact same fields (error: null,registrationCompletedAt,registrationLastCheckedAt,status: "complete") via a separate inlineupdateOnboardingcall. Now that this helper exists,pollRegistrationshould call it instead of duplicating the logic, so both completion paths stay in sync going forward.♻️ Proposed fix
const now = new Date(); const registered = await options.chain.isUserRegistered(row.address); if (registered) { - await updateOnboarding(db, row.userId, { - error: null, - registrationCompletedAt: now, - registrationLastCheckedAt: now, - status: "complete", - }); + await markOnboardingRegistered(db, row.userId); return; }🤖 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 `@services/api/src/onboarding/service.ts` around lines 519 - 530, Update the registered branch of pollRegistration to call markOnboardingRegistered(db, userId) instead of its duplicate inline updateOnboarding call, preserving the existing completion behavior and removing the repeated field assignments.services/api/src/config.ts (1)
182-192: 🧹 Nitpick | 🔵 TrivialConfirm self-custody deployments explicitly set this flag.
The new default (
false) means any deployment that doesn't explicitly setONBOARDING_REQUIRE_OPERATOR_REGISTRATION=truewill silently skip operator-registration polling — including any existing self-custody deployment whose env config wasn't updated for this change. Worth confirming deployment configs/runbooks for self-custody environments are updated to set this explicitly, since the security gate (operator eERC registration) would otherwise be bypassed without any warning at startup.🤖 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 `@services/api/src/config.ts` around lines 182 - 192, Update self-custody deployment configurations and operational runbooks to explicitly set ONBOARDING_REQUIRE_OPERATOR_REGISTRATION=true, and add startup validation or a warning for self-custody deployments when this flag is false. Preserve the false default for managed-custody deployments while ensuring self-custody cannot silently bypass operator-registration polling.
🤖 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.
Nitpick comments:
In `@services/api/src/config.ts`:
- Around line 182-192: Update self-custody deployment configurations and
operational runbooks to explicitly set
ONBOARDING_REQUIRE_OPERATOR_REGISTRATION=true, and add startup validation or a
warning for self-custody deployments when this flag is false. Preserve the false
default for managed-custody deployments while ensuring self-custody cannot
silently bypass operator-registration polling.
In `@services/api/src/onboarding/service.ts`:
- Around line 519-530: Update the registered branch of pollRegistration to call
markOnboardingRegistered(db, userId) instead of its duplicate inline
updateOnboarding call, preserving the existing completion behavior and removing
the repeated field assignments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b61781df-55ea-4d39-a189-a35d2505ea8f
📒 Files selected for processing (4)
services/api/src/config.tsservices/api/src/onboarding/service.tsservices/api/test/api.test.tsservices/api/test/orgs.test.ts
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // register their own eERC identity. | ||
| ONBOARDING_REQUIRE_OPERATOR_REGISTRATION: z | ||
| .enum(["true", "false"]) | ||
| .default("false") |
There was a problem hiding this comment.
When an existing self-custody deployment upgrades without setting the new env var, the default false value sends gas_dripped and awaiting_registration rows directly to complete. Those operators can be treated as onboarded before their eERC registration is verified, so later encrypted-balance flows can fail after the UI has left onboarding.
Prompt To Fix With AI
This is a comment left during a code review.
Path: services/api/src/config.ts
Line: 191
Comment:
**Self-Custody Default Skips Registration**
When an existing self-custody deployment upgrades without setting the new env var, the default `false` value sends `gas_dripped` and `awaiting_registration` rows directly to `complete`. Those operators can be treated as onboarded before their eERC registration is verified, so later encrypted-balance flows can fail after the UI has left onboarding.
How can I resolve this? If you propose a fix, please make it concise.
1 participant
Problem
Console onboarding on BenzoNet L1 hung forever at
awaiting_registration. The UI wizard gates treasury provisioning behind KYB completion, so the whole flow deadlocked.Root cause
Managed onboarding polled
isUserRegistered(operator), but nothing ever registers the operator EOA:In managed custody the operator is an admin that never holds an encrypted balance (the managed treasury does, and it is eERC-registered separately), so waiting on operator registration is both impossible and unnecessary.
Fix
New
ONBOARDING_REQUIRE_OPERATOR_REGISTRATIONflag (defaultfalse= managed). When false, onboarding completes once KYC + allowlist + gas land, and any row already parked inawaiting_registrationis completed rather than polled. Settrueonly for a self-custody deployment where operators register themselves.Verification
true(unchanged behavior).registered:true) → dashboard.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Greptile Summary
This PR changes onboarding so managed custody no longer waits for operator eERC registration. The main changes are:
ONBOARDING_REQUIRE_OPERATOR_REGISTRATIONconfig flag.awaiting_registrationrows when operator registration is disabled.Confidence Score: 5/5
This looks safe to merge after confirming the new default is acceptable for all deployed environments.
services/api/src/config.ts
Important Files Changed
gas_drippedandawaiting_registration.Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(api): complete managed onboarding wi..." | Re-trigger Greptile