feat(billing): create a wallet for every account and make start-trial idempotent - #3513
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3513 +/- ##
==========================================
- Coverage 73.85% 73.42% -0.43%
==========================================
Files 1139 1078 -61
Lines 29704 27794 -1910
Branches 7445 7042 -403
==========================================
- Hits 21939 20409 -1530
+ Misses 6854 6520 -334
+ Partials 911 865 -46
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesWallet activation lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts (1)
108-114: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrial-window/expiration logic duplicated across two services and already diverging. Both
ManagedUserWalletService.refillWalletFeesandRefillService.refillWalletFeesindependently computetrialStartedAt = activatedAt ?? createdAt, an "in trial window" boolean, and an expiration date — the same rule maintained in two places with different comparison styles.
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts#L108-L114: extract this trial-window/expiration calculation into a small shared helper (e.g. onManagedUserWalletServiceor a standalone util) that both services call.apps/api/src/billing/services/refill/refill.service.ts#L56-L61: replace this duplicate calc (currently using rawDate >and redundanttrialStartedAt &&guards) with a call to the shared helper instead of maintaining a second, subtly different implementation.🤖 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 `@apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts` around lines 108 - 114, Extract the trial-window and expiration calculation from ManagedUserWalletService.refillWalletFees into a shared helper, preserving the activatedAt-or-createdAt start time and existing date-fns comparison behavior. Update apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts:108-114 to use the helper, and replace the duplicate calculation in apps/api/src/billing/services/refill/refill.service.ts:56-61 with the same helper, removing its raw Date comparison and redundant guards.
🤖 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 `@apps/api/drizzle/0033_long_solo.sql`:
- Line 2: Update the backfill statement so the timestamp without time zone in
created_at is explicitly interpreted as UTC before assigning it to timestamptz
activated_at; preserve the existing non-null row filter and ensure the migration
is independent of the session TimeZone.
In `@apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts`:
- Around line 85-93: Update claimActivation to enforce wallet authorization at
the mutation site rather than relying on callers resolving the wallet first.
Thread the scoped repository or authorization context into claimActivation, or
perform an explicit ownership/access check before the update, while preserving
the existing atomic activatedAt condition and return behavior.
---
Nitpick comments:
In
`@apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts`:
- Around line 108-114: Extract the trial-window and expiration calculation from
ManagedUserWalletService.refillWalletFees into a shared helper, preserving the
activatedAt-or-createdAt start time and existing date-fns comparison behavior.
Update
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts:108-114
to use the helper, and replace the duplicate calculation in
apps/api/src/billing/services/refill/refill.service.ts:56-61 with the same
helper, removing its raw Date comparison and redundant guards.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f3e783fb-82ac-420b-b87a-9fe1f5817ba7
⛔ Files ignored due to path filters (1)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (29)
apps/api/drizzle/0033_long_solo.sqlapps/api/drizzle/meta/0033_snapshot.jsonapps/api/drizzle/meta/_journal.jsonapps/api/src/billing/model-schemas/user-wallet/user-wallet.schema.tsapps/api/src/billing/repositories/user-wallet/user-wallet.repository.integration.tsapps/api/src/billing/repositories/user-wallet/user-wallet.repository.tsapps/api/src/billing/routes/start-trial/start-trial.router.tsapps/api/src/billing/services/managed-signer/managed-signer.service.tsapps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.spec.tsapps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.tsapps/api/src/billing/services/refill/refill.service.spec.tsapps/api/src/billing/services/refill/refill.service.tsapps/api/src/billing/services/trial-validation/trial-validation.service.spec.tsapps/api/src/billing/services/trial-validation/trial-validation.service.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.tsapps/api/src/billing/services/wallet-reader/wallet-reader.service.spec.tsapps/api/src/billing/services/wallet-reader/wallet-reader.service.tsapps/api/src/deployment/repositories/deployment-setting/deployment-setting.repository.tsapps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.tsapps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.tsapps/api/src/user/repositories/user/user.repository.integration.tsapps/api/src/user/repositories/user/user.repository.tsapps/api/src/user/services/user/user.service.integration.tsapps/api/src/user/services/user/user.service.spec.tsapps/api/src/user/services/user/user.service.tsapps/api/test/functional/wallets-refill.spec.tsapps/api/test/seeders/auto-top-up-deployment.seeder.tsapps/api/test/seeders/user-wallet.seeder.ts
- Backfill activated_at (timestamptz) by interpreting created_at (timestamp without time zone) as UTC, so migration is independent of session TimeZone. - claimActivation enforces ability at the mutation site via whereAccessibleBy, matching updateById/deleteById (passes through when no ability is set). - RefillService.refillWalletFees delegates to ManagedUserWalletService .refillWalletFees, removing the duplicated trial-window/expiration calc.
Contributor
Author
|
Addressed the CodeRabbit review in d669721:
Unit tests (52) pass, lint clean, typecheck clean for the changed files. Functional/integration coverage runs in CI. |
ygrishajev
approved these changes
Jul 24, 2026
This was referenced Jul 24, 2026
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
2 participants
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.
Why
Managed wallets were only created when a trial started, and provisioning bundled row creation, address derivation, and slow on-chain grant authorization into one non-idempotent step. An interrupted or concurrent start-trial left half-provisioned rows with
NULLaddresses that surfaced to clients (source of Sentry 7542732522) or, on failure, deleted the row entirely.Closes CON-744
Related to CON-743 (frontend guard, #3512)
What
Every registered account gets a wallet row with a derived address; a new
activated_atcolumn becomes the activation signal (replaces "row exists"), so all client-facing semantics stay unchanged.user_wallets.activated_at timestamptz+ backfillactivated_at = created_atfor rows with an address. Additive and non-blocking;NULL-address orphans stay unactivated and self-heal on next start-trial.UserService.registerUseridempotently ensures a wallet (row + derived address, no chain transactions) on every login — lazy backfill for existing users. Failures are logged and never block login.getOrCreate+ address heal, then a conditional-UPDATEclaim onactivated_at. The concurrent loser gets a 409 (absorbed by the client's mutation retry). On grant failure only the activation stamp rolls back — the row and address are kept (previously the row was deleted).TrialStartedfires exactly once, on a successful claim.GET /v1/walletsreturns only activated wallets, so a wallet without grants can never reach the client.RefillServicestampsactivated_at).createdAtas "trial started" were updated to the activation signal:activated_atinstead ofcreatedAt(registration date no longer approximates trial start).Verified locally against a real Postgres + built server: migration backfill via the production migrator; start-trial on a fresh account creates the row with a real derived address; a failed grant returns 500 while keeping the row and unsetting the claim; retries reuse the same row;
/v1/walletsstays empty until activation and returns the wallet after; re-running start-trial on an activated wallet returns 200 with the same state. The claim race (exactly one winner among 5 concurrent claims) is covered by an integration test on real Postgres.Summary by CodeRabbit
activatedAt) and updated wallet/refill/trial flows to focus on activated wallets.activatedAtwhen available.409response for trial provisioning already in progress.