fix(managed-wallet): ensure fee authorization sync with blockchain and auto-refill for trial wallets #2444
Conversation
📝 WalkthroughWalkthroughAdds NOT NULL constraints to user_wallets timestamps and threads ManagedSignerService through billing flows to fetch live allowances, auto-refill fees via ManagedUserWalletService when below threshold, and updates call sites and tests to pass signer into wallet authorization methods. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant ManagedSigner as ManagedSignerService
participant Balances as BalancesService
participant UserWallet as ManagedUserWalletService
participant Chain as Blockchain
Client->>ManagedSigner: validateBalances(userWallet, msgSet?)
ManagedSigner->>Balances: retrieveAndCalcFeeLimit()
Balances-->>ManagedSigner: feeAllowance
ManagedSigner->>Chain: fetchLiveAllowances()
Chain-->>ManagedSigner: feeAllowance, deploymentAllowance
alt feeAllowance < threshold
ManagedSigner->>UserWallet: refillWalletFees(ManagedSigner, userWallet)
activate UserWallet
UserWallet->>UserWallet: compute trial expiry & limits
UserWallet->>UserWallet: authorizeSpending(ManagedSigner, {limits})
UserWallet-->>ManagedSigner: refill authorized/executed
deactivate UserWallet
ManagedSigner->>Balances: retrieveAndCalcFeeLimit()
Balances-->>ManagedSigner: updatedFeeAllowance
end
ManagedSigner->>ManagedSigner: assert fee/deployment availability
ManagedSigner-->>Client: proceed or throw error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
ygrishajev
force-pushed
the
feature/fees
branch
from
e6243f5 to
7ed8c96
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/api/src/billing/services/managed-signer/managed-signer.service.ts (1)
180-183: Consider adding error handling around the refill operation.If
refillWalletFeesfails, the assertion on line 185 may produce a confusing "Not enough funds" error when the actual issue was a refill failure. Consider wrapping the refill logic in a try-catch to provide clearer error context or let the refill error propagate directly.🔎 Suggested approach
if (feeAllowance < this.billingConfigService.get("FEE_ALLOWANCE_REFILL_THRESHOLD")) { - await this.managedUserWalletService.refillWalletFees(this, userWallet); - feeAllowance = await this.balancesService.retrieveAndCalcFeeLimit(userWallet); + try { + await this.managedUserWalletService.refillWalletFees(this, userWallet); + feeAllowance = await this.balancesService.retrieveAndCalcFeeLimit(userWallet); + } catch (error: any) { + throw new Error(`Failed to refill wallet fees for wallet ${userWallet.id}: ${error.message}`, { cause: error }); + } }apps/api/src/billing/services/refill/refill.service.spec.ts (1)
93-124: Consider consolidating duplicatesetupfunctions.Both
topUpWalletandreduceWalletBalancedescribe blocks have nearly identicalsetupfunctions. Per coding guidelines, thesetupfunction should be at the bottom of the rootdescribeblock. Consolidating these would reduce duplication and improve maintainability.🔎 Suggested consolidation
Move the
setupfunction to the rootdescribe(RefillService.name, ...)block:describe(RefillService.name, () => { describe("topUpWallet", () => { // ... tests ... - function setup() { ... } }); describe("reduceWalletBalance", () => { // ... tests ... - function setup() { ... } }); + + function setup() { + const billingConfig = mock<BillingConfig>(); + const userWalletRepository = mock<UserWalletRepository>(); + const managedUserWalletService = mock<ManagedUserWalletService>(); + const managedSignerService = mock<ManagedSignerService>(); + const balancesService = mock<BalancesService>(); + const walletInitializerService = mock<WalletInitializerService>(); + const analyticsService = mock<AnalyticsService>(); + + billingConfig.FEE_ALLOWANCE_REFILL_AMOUNT = 1000; + + const service = new RefillService( + billingConfig, + userWalletRepository, + managedUserWalletService, + managedSignerService, + balancesService, + walletInitializerService, + analyticsService + ); + + return { + service, + billingConfig, + userWalletRepository, + managedUserWalletService, + managedSignerService, + balancesService, + walletInitializerService, + analyticsService + }; + } });Also applies to: 196-227
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/api/drizzle/0026_chief_mysterio.sqlapps/api/drizzle/meta/0026_snapshot.jsonapps/api/drizzle/meta/_journal.jsonapps/api/src/billing/model-schemas/user-wallet/user-wallet.schema.tsapps/api/src/billing/services/managed-signer/managed-signer.service.spec.tsapps/api/src/billing/services/managed-signer/managed-signer.service.tsapps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.tsapps/api/src/billing/services/provider-cleanup/provider-cleanup.service.tsapps/api/src/billing/services/refill/refill.service.spec.tsapps/api/src/billing/services/refill/refill.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/deployment/services/stale-managed-deployments-cleaner/stale-managed-deployments-cleaner.service.tsapps/api/test/functional/wallets-refill.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/api/src/billing/model-schemas/user-wallet/user-wallet.schema.tsapps/api/src/deployment/services/stale-managed-deployments-cleaner/stale-managed-deployments-cleaner.service.tsapps/api/test/functional/wallets-refill.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.tsapps/api/src/billing/services/managed-signer/managed-signer.service.tsapps/api/src/billing/services/refill/refill.service.tsapps/api/src/billing/services/managed-signer/managed-signer.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/provider-cleanup/provider-cleanup.service.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/api/test/functional/wallets-refill.spec.tsapps/api/src/billing/services/managed-signer/managed-signer.service.spec.tsapps/api/src/billing/services/refill/refill.service.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: ygrishajev
Repo: akash-network/console PR: 2274
File: apps/api/drizzle/0023_sad_adam_warlock.sql:1-1
Timestamp: 2025-11-28T09:17:28.832Z
Learning: In the Akash Network console billing system, the `user_wallets.user_id` column is required (NOT NULL). All user wallets must be associated with a user ID, and there are no anonymous wallets without a user_id in production.
📚 Learning: 2025-11-28T09:17:28.832Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2274
File: apps/api/drizzle/0023_sad_adam_warlock.sql:1-1
Timestamp: 2025-11-28T09:17:28.832Z
Learning: In the Akash Network console billing system, the `user_wallets.user_id` column is required (NOT NULL). All user wallets must be associated with a user ID, and there are no anonymous wallets without a user_id in production.
Applied to files:
apps/api/src/billing/model-schemas/user-wallet/user-wallet.schema.tsapps/api/drizzle/0026_chief_mysterio.sql
📚 Learning: 2025-12-29T12:12:27.886Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2432
File: apps/api/src/billing/services/tx-manager/tx-manager.service.ts:116-118
Timestamp: 2025-12-29T12:12:27.886Z
Learning: In the Akash Console codebase (akash-network/console), v1 wallets are only used for read-only purposes such as retrieving addresses. All transaction signing operations use v2 wallets exclusively. Therefore, methods like `getDerivedWalletAddress` need to accept `WalletOptions` to support both v1 and v2 address lookups, while transaction signing methods like `signAndBroadcastWithDerivedWallet` correctly default to v2 without requiring a wallet version parameter.
Applied to files:
apps/api/test/functional/wallets-refill.spec.ts
📚 Learning: 2025-11-25T17:45:44.790Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-11-25T17:45:44.790Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test
Applied to files:
apps/api/src/billing/services/managed-signer/managed-signer.service.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/api/src/billing/services/managed-signer/managed-signer.service.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts
📚 Learning: 2025-12-29T13:20:43.626Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2422
File: apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts:124-128
Timestamp: 2025-12-29T13:20:43.626Z
Learning: In the wallet-settings service (apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts), auto-reload jobs are not eagerly canceled when autoReloadEnabled is disabled. Instead, jobs check the autoReloadEnabled flag at execution time and complete early if disabled. This design avoids concurrency issues that could occur with eager cancellation.
Applied to files:
apps/api/src/billing/services/managed-signer/managed-signer.service.spec.ts
📚 Learning: 2025-10-15T16:39:55.348Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.348Z
Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.
Applied to files:
apps/api/src/billing/services/refill/refill.service.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts
📚 Learning: 2025-10-31T11:26:42.138Z
Learnt from: stalniy
Repo: akash-network/console PR: 2138
File: apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts:379-382
Timestamp: 2025-10-31T11:26:42.138Z
Learning: In TypeScript/JavaScript, the pattern of checking a cached value and then performing an async operation to fetch it without proper synchronization is race condition unsafe:
```typescript
private async getAddress() {
if (!this.address) {
this.address = await this.wallet.getFirstAddress();
}
return this.address;
}
```
Multiple concurrent calls can all pass the `if (!this.address)` check before any of them sets the value, leading to duplicate async operations. This should be flagged as a race condition. Proper synchronization (mutex, atomic promise caching, or guaranteed single-threaded execution) is required.
Applied to files:
apps/api/src/billing/services/refill/refill.service.spec.ts
🧬 Code graph analysis (1)
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.ts (1)
apps/api/src/billing/services/managed-signer/managed-signer.service.ts (1)
userWallet(169-187)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (21)
apps/api/src/billing/services/managed-signer/managed-signer.service.spec.ts (6)
15-15: LGTM!The new imports are correctly added for the dependencies introduced in this PR:
BillingConfigService,ManagedUserWalletService, andmockConfigServiceutility.Also applies to: 17-17, 29-29
76-76: LGTM!The test mocks have been updated to include the new
retrieveAndCalcFeeLimitandretrieveDeploymentLimitmethods, which now return 0 to properly simulate insufficient funds scenarios.Also applies to: 96-97
595-596: LGTM!The setup function input type correctly extends to include the new
BalancesServicemethods for testing different allowance scenarios.
610-614: LGTM!The default mock values for
retrieveAndCalcFeeLimit(1,000,000 uAKT) andretrieveDeploymentLimit(5,000,000 uAKT) are reasonable defaults that allow tests to pass unless explicitly testing insufficient funds scenarios.
642-649: LGTM!The new service mocks are properly configured:
billingConfigServiceusesmockConfigServicewith appropriate config valuesmanagedUserWalletServicehas a stub forrefillWalletFeesBoth follow the jest-mock-extended pattern as required by the coding guidelines.
656-671: LGTM!The
ManagedSignerServiceconstructor has been updated to include the new dependencies (billingConfigServiceat line 658, andmanagedUserWalletServiceat line 670). The ordering and wiring are correct.apps/api/drizzle/meta/_journal.json (1)
187-193: LGTM!The migration journal entry is correctly formatted and follows the expected Drizzle ORM pattern. The entry properly sequences after idx 25 and documents the "0026_chief_mysterio" migration.
apps/api/src/billing/services/refill/refill.service.ts (2)
10-10: LGTM!The
ManagedSignerServicedependency has been correctly added via import and constructor injection. This refactoring helps avoid the circular dependency issue mentioned in the PR objectives.Also applies to: 24-24
53-53: LGTM!All three
authorizeSpendingcalls have been consistently updated to passthis.managedSignerServiceas the first parameter. This aligns with the refactored method signature inManagedUserWalletService.Also applies to: 74-74, 104-104
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.ts (1)
6-6: LGTM! ManagedSignerService integration is clean.The changes correctly inject and use ManagedSignerService to support signer-aware trial spending authorization, aligning with the PR's goal of syncing fee authorization with the blockchain.
Also applies to: 16-16, 30-30
apps/api/test/functional/wallets-refill.spec.ts (1)
31-35: LGTM! Test correctly reflects the new authorizeSpending signature.The test properly resolves ManagedSignerService from the container and passes it as the first argument to authorizeSpending, consistent with the refactored API.
apps/api/drizzle/meta/0026_snapshot.json (1)
50-63: LGTM! Schema snapshot correctly reflects NOT NULL constraints.The generated snapshot properly documents the NOT NULL constraints on
created_atandupdated_atcolumns in theuser_walletstable, with appropriate default values ofnow().apps/api/src/deployment/services/stale-managed-deployments-cleaner/stale-managed-deployments-cleaner.service.ts (1)
68-77: LGTM! Error recovery path correctly uses signer-aware authorization.The updated authorizeSpending call properly passes
this.managedSignerServiceas the first argument before retrying the transaction, ensuring proper fee authorization when the initial attempt fails.apps/api/src/billing/services/managed-signer/managed-signer.service.ts (2)
35-35: LGTM! New dependencies correctly injected.BillingConfigService and ManagedUserWalletService are properly added to support the auto-refill logic. The circular dependency is elegantly avoided by passing the signer as a method parameter to
refillWalletFeesrather than relying solely on constructor injection.Also applies to: 47-47
171-176: LGTM! Fetching balances from blockchain addresses stale data issue.The refactored logic correctly retrieves current fee and deployment allowances from the blockchain instead of relying on potentially stale database values, directly addressing the PR's core objective.
apps/api/src/billing/services/provider-cleanup/provider-cleanup.service.ts (1)
62-76: LGTM! Signer parameter correctly passed for fee authorization retry.The error handling flow correctly passes
this.managedSignerServiceas the first argument toauthorizeSpending, aligning with the refactored method signature. The retry pattern after fee authorization is appropriate for handling stale allowance scenarios.apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts (1)
114-164: LGTM! Test setup correctly wires mocks for the new signer-based flow.The
setupfunction properly:
- Uses
jest-mock-extendedfor creating mocks (per coding guidelines)- Registers
ManagedSignerServicemock for the new dependency- Registers
TYPE_REGISTRYwith aRegistryinstance- Is positioned at the bottom of the describe block
The test assertions correctly verify that
createAndAuthorizeTrialSpendingreceives theManagedSignerServiceinstance as the first parameter.apps/api/src/billing/services/refill/refill.service.spec.ts (1)
30-33: LGTM! Assertions correctly verify signer parameter.All
authorizeSpendingassertions have been properly updated to verify thatmanagedSignerServiceis passed as the first argument, aligning with the new method signature.Also applies to: 53-56, 79-86, 144-147, 164-167
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts (3)
40-54: LGTM! Clean separation of signer dependency.Passing
signeras a parameter instead of injecting it via constructor effectively breaks the circular dependency mentioned in the PR objectives. The method correctly delegates toauthorizeSpendingwith the signer.
63-85: LGTM! Signer parameter correctly propagated through authorization chain.The
authorizeSpendingmethod cleanly passes the signer to bothauthorizeDeploymentSpendingandauthorizeFeeSpending, maintaining consistency across the authorization flow.
110-126: LGTM! Private authorization methods correctly use signer.Both
authorizeFeeSpendingandauthorizeDeploymentSpendingproperly usesigner.executeFundingTxinstead of a constructor-injected dependency, completing the signer-parameterized refactor.
apps/api/src/billing/services/managed-signer/managed-signer.service.ts
Outdated
Show resolved
Hide resolved
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (77.50%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2444 +/- ##
==========================================
- Coverage 51.31% 51.06% -0.26%
==========================================
Files 1069 1059 -10
Lines 29438 29090 -348
Branches 6508 6481 -27
==========================================
- Hits 15106 14854 -252
- Misses 13927 13946 +19
+ Partials 405 290 -115
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
ygrishajev
force-pushed
the
feature/fees
branch
from
7ed8c96 to
e42f5f3
Compare
…d auto-refill for trial wallets - Always fetch fee allowance from blockchain in ManagedSignerService to ensure accuracy, as database values may be out of sync - Fetch deployment allowance from blockchain only when deployment message is present, otherwise use cached database value - Automatically refill fee authorization for eligible trial wallets when fee allowance is below FEE_ALLOWANCE_REFILL_THRESHOLD - Refactor ManagedUserWalletService to accept ManagedSignerService as parameter instead of constructor dependency to avoid circular dependency - Add refillWalletFees method to ManagedUserWalletService for centralized fee refill logic - Update all services (RefillService, WalletInitializerService, ProviderCleanupService, StaleManagedDeploymentsCleanerService) to pass ManagedSignerService to authorizeSpending calls - Update all test files to reflect new method signatures and ensure proper mock setup - Make createdAt and updatedAt fields not null in user wallet schema This prevents transaction failures caused by out-of-sync fee allowances in the database by proactively checking and authorizing fees from the blockchain when necessary, especially for trial users. refs #2338
ygrishajev
force-pushed
the
feature/fees
branch
from
e42f5f3 to
390d9b2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/api/drizzle/0026_chief_mysterio.sql (1)
1-2: Ensure existing NULL values are migrated before applying NOT NULL constraints.As noted in the previous review, this migration may fail if existing rows contain NULL values in
created_atorupdated_at. Update any NULL rows before altering the columns.apps/api/src/billing/services/managed-signer/managed-signer.service.ts (1)
177-183: Incorrect threshold comparison for fee refill (already flagged).Line 179 compares
feeAllowanceagainstFEE_ALLOWANCE_REFILL_AMOUNT, but it should compare againstFEE_ALLOWANCE_REFILL_THRESHOLD. The threshold is the trigger point for refills, while the amount is the target balance after refilling.🔎 Proposed fix
- if (feeAllowance < this.billingConfigService.get("FEE_ALLOWANCE_REFILL_AMOUNT")) { + if (feeAllowance < this.billingConfigService.get("FEE_ALLOWANCE_REFILL_THRESHOLD")) { await this.managedUserWalletService.refillWalletFees(this, userWallet); feeAllowance = await this.balancesService.retrieveAndCalcFeeLimit(userWallet); }apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts (1)
87-108: Add guard clause for nullable address (already flagged).Line 102 uses a non-null assertion on
userWallet.address!which can be null per the database schema. This is an existing issue that should be addressed by adding a guard clause at the beginning of the method.🔎 Proposed fix
async refillWalletFees(signer: ManagedSignerService, userWallet: UserWalletOutput) { + if (!userWallet.address) { + throw new Error("Managed user wallet missing address"); + } + const trialWindowStart = subDays(new Date(), this.config.TRIAL_ALLOWANCE_EXPIRATION_DAYS); const isInTrialWindow = userWallet.isTrialing && isAfter(userWallet.createdAt, trialWindowStart); const expiration = isInTrialWindow ? addDays(userWallet.createdAt, this.config.TRIAL_ALLOWANCE_EXPIRATION_DAYS) : undefined; const fees = this.config.FEE_ALLOWANCE_REFILL_AMOUNT; await this.authorizeSpending(signer, { - address: userWallet.address!, + address: userWallet.address, limits: { fees }, expiration }); }
🧹 Nitpick comments (2)
apps/api/test/functional/wallets-refill.spec.ts (1)
31-35: Consider resolvingManagedSignerServiceonce outside the loop.
ManagedSignerServiceis currently resolved from the container once per wallet iteration. Moving the resolution outside themapcallback or into thesetupfunction would avoid redundant container lookups.🔎 Proposed refactor
const { managedUserWalletService, config, walletController, walletService, userWalletRepository } = setup(); + const managedSignerService = container.resolve(ManagedSignerService); const NUMBER_OF_WALLETS = 5; const prepareRecords = Array.from({ length: NUMBER_OF_WALLETS }).map(async (_, index) => { const records = await walletService.createUserAndWallet(); const { user, token } = records; const { wallet } = records; let walletRecord = await userWalletRepository.findById(wallet.id); expect(wallet.creditAmount).toBe(config.TRIAL_DEPLOYMENT_ALLOWANCE_AMOUNT); expect(walletRecord?.feeAllowance).toBe(config.TRIAL_FEES_ALLOWANCE_AMOUNT); const limits = { fees: config.FEE_ALLOWANCE_REFILL_THRESHOLD }; - const managedSignerService = container.resolve(ManagedSignerService); await managedUserWalletService.authorizeSpending(managedSignerService, {apps/api/src/billing/services/managed-signer/managed-signer.service.spec.ts (1)
585-676: Consider adding tests for the auto-refill behavior.While the test setup correctly includes mocks for
billingConfigServiceandmanagedUserWalletService.refillWalletFees, there are no tests that verify the auto-refill behavior mentioned in the PR objectives (automatically refilling fee authorization for trial wallets when below threshold).Consider adding test cases that verify:
refillWalletFeesis called when fee allowance falls belowFEE_ALLOWANCE_REFILL_THRESHOLDfor eligible trial wallets- Auto-refill is skipped for non-trial wallets or when allowance is above threshold
If this behavior is tested elsewhere (e.g., in
ManagedUserWalletServicetests or integration tests), this suggestion can be safely ignored.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/api/drizzle/0026_chief_mysterio.sqlapps/api/drizzle/meta/0026_snapshot.jsonapps/api/drizzle/meta/_journal.jsonapps/api/src/billing/model-schemas/user-wallet/user-wallet.schema.tsapps/api/src/billing/services/managed-signer/managed-signer.service.spec.tsapps/api/src/billing/services/managed-signer/managed-signer.service.tsapps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.tsapps/api/src/billing/services/provider-cleanup/provider-cleanup.service.tsapps/api/src/billing/services/refill/refill.service.spec.tsapps/api/src/billing/services/refill/refill.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/deployment/services/stale-managed-deployments-cleaner/stale-managed-deployments-cleaner.service.tsapps/api/test/functional/wallets-refill.spec.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/api/src/billing/model-schemas/user-wallet/user-wallet.schema.ts
- apps/api/src/billing/services/provider-cleanup/provider-cleanup.service.ts
- apps/api/drizzle/meta/_journal.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/api/src/billing/services/refill/refill.service.tsapps/api/src/deployment/services/stale-managed-deployments-cleaner/stale-managed-deployments-cleaner.service.tsapps/api/test/functional/wallets-refill.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/wallet-initializer/wallet-initializer.service.tsapps/api/src/billing/services/managed-signer/managed-signer.service.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/billing/services/managed-signer/managed-signer.service.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/api/test/functional/wallets-refill.spec.tsapps/api/src/billing/services/refill/refill.service.spec.tsapps/api/src/billing/services/managed-signer/managed-signer.service.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: ygrishajev
Repo: akash-network/console PR: 2274
File: apps/api/drizzle/0023_sad_adam_warlock.sql:1-1
Timestamp: 2025-11-28T09:17:28.832Z
Learning: In the Akash Network console billing system, the `user_wallets.user_id` column is required (NOT NULL). All user wallets must be associated with a user ID, and there are no anonymous wallets without a user_id in production.
📚 Learning: 2025-11-28T09:17:28.832Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2274
File: apps/api/drizzle/0023_sad_adam_warlock.sql:1-1
Timestamp: 2025-11-28T09:17:28.832Z
Learning: In the Akash Network console billing system, the `user_wallets.user_id` column is required (NOT NULL). All user wallets must be associated with a user ID, and there are no anonymous wallets without a user_id in production.
Applied to files:
apps/api/drizzle/0026_chief_mysterio.sqlapps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts
📚 Learning: 2025-12-29T12:12:27.886Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2432
File: apps/api/src/billing/services/tx-manager/tx-manager.service.ts:116-118
Timestamp: 2025-12-29T12:12:27.886Z
Learning: In the Akash Console codebase (akash-network/console), v1 wallets are only used for read-only purposes such as retrieving addresses. All transaction signing operations use v2 wallets exclusively. Therefore, methods like `getDerivedWalletAddress` need to accept `WalletOptions` to support both v1 and v2 address lookups, while transaction signing methods like `signAndBroadcastWithDerivedWallet` correctly default to v2 without requiring a wallet version parameter.
Applied to files:
apps/api/test/functional/wallets-refill.spec.tsapps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts
📚 Learning: 2025-10-31T11:26:42.138Z
Learnt from: stalniy
Repo: akash-network/console PR: 2138
File: apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts:379-382
Timestamp: 2025-10-31T11:26:42.138Z
Learning: In TypeScript/JavaScript, the pattern of checking a cached value and then performing an async operation to fetch it without proper synchronization is race condition unsafe:
```typescript
private async getAddress() {
if (!this.address) {
this.address = await this.wallet.getFirstAddress();
}
return this.address;
}
```
Multiple concurrent calls can all pass the `if (!this.address)` check before any of them sets the value, leading to duplicate async operations. This should be flagged as a race condition. Proper synchronization (mutex, atomic promise caching, or guaranteed single-threaded execution) is required.
Applied to files:
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.tsapps/api/src/billing/services/refill/refill.service.spec.ts
📚 Learning: 2025-10-15T16:39:55.348Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.348Z
Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.
Applied to files:
apps/api/src/billing/services/refill/refill.service.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts
📚 Learning: 2025-11-25T17:45:44.790Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-11-25T17:45:44.790Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test
Applied to files:
apps/api/src/billing/services/managed-signer/managed-signer.service.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/api/src/billing/services/managed-signer/managed-signer.service.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts
📚 Learning: 2025-12-29T13:20:43.626Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2422
File: apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts:124-128
Timestamp: 2025-12-29T13:20:43.626Z
Learning: In the wallet-settings service (apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts), auto-reload jobs are not eagerly canceled when autoReloadEnabled is disabled. Instead, jobs check the autoReloadEnabled flag at execution time and complete early if disabled. This design avoids concurrency issues that could occur with eager cancellation.
Applied to files:
apps/api/src/billing/services/managed-signer/managed-signer.service.spec.ts
🧬 Code graph analysis (4)
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts (3)
apps/api/src/billing/services/managed-signer/managed-signer.service.ts (1)
userWallet(170-186)apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts (1)
UserWalletOutput(18-22)apps/api/src/billing/services/rpc-message-service/rpc-message.service.ts (1)
SpendingAuthorizationMsgOptions(16-22)
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.ts (1)
apps/api/src/billing/services/managed-signer/managed-signer.service.ts (1)
userWallet(170-186)
apps/api/src/billing/services/managed-signer/managed-signer.service.spec.ts (1)
apps/api/test/mocks/config-service.mock.ts (1)
mockConfigService(16-25)
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts (1)
apps/api/src/billing/providers/type-registry.provider.ts (1)
TYPE_REGISTRY(34-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (21)
apps/api/src/billing/services/refill/refill.service.ts (1)
10-10: LGTM!The changes correctly thread
ManagedSignerServicethrough allauthorizeSpendingcalls. The refactoring aligns with the PR's objective to pass the signer as a method parameter, avoiding circular dependencies while enabling blockchain-backed fee authorization.Also applies to: 24-24, 53-53, 74-74, 104-104
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.ts (1)
6-6: LGTM!The injection and usage of
ManagedSignerServicefollows the same pattern as other services in this PR. The signer is correctly passed tocreateAndAuthorizeTrialSpendingfor blockchain-backed authorization.Also applies to: 16-16, 30-30
apps/api/src/deployment/services/stale-managed-deployments-cleaner/stale-managed-deployments-cleaner.service.ts (2)
21-30: LGTM! Constructor properly updated with ManagedSignerService dependency.The addition of
ManagedSignerServiceto the constructor is consistent with the signer-based authorization pattern introduced across the PR.
64-81: LGTM! Error handling correctly updated to pass signer.The error recovery flow properly passes
this.managedSignerServicetoauthorizeSpendingand retries the transaction after authorization, consistent with the signer-based pattern throughout the PR.apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts (3)
1-1: LGTM! Imports properly updated for new dependencies.The added imports for
Registry,TYPE_REGISTRY, andManagedSignerServiceare necessary to support the signer-based authorization flow in the tests.Also applies to: 8-8, 15-15
25-111: LGTM! Test assertions correctly updated for signer-based flow.All test cases properly pass
ManagedSignerServiceas the first argument tocreateAndAuthorizeTrialSpending, and assertions verify the correct parameters. The tests follow coding guidelines by usingjest-mock-extendedand the setup function pattern.
114-164: LGTM! Setup function properly registers all required dependencies.The setup function correctly registers
TYPE_REGISTRY,ManagedUserWalletService, andManagedSignerServicein the DI container, following the established test patterns.apps/api/src/billing/services/managed-signer/managed-signer.service.ts (2)
33-48: LGTM! Constructor properly updated with new dependencies.The addition of
BillingConfigServiceandManagedUserWalletServiceto the constructor is correct and necessary for the auto-refill logic.
158-175: LGTM! Chain-based allowance fetching correctly implemented.The changes to always fetch fee allowance from the blockchain and conditionally fetch deployment allowance address the core PR objective of preventing transaction failures from out-of-sync allowances.
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts (3)
1-14: LGTM! Imports properly updated for new functionality.The addition of date-fns utilities and
UserWalletOutputtype are necessary for the newrefillWalletFeesmethod, and the type-only import ofManagedSignerServicecorrectly avoids circular dependencies.
40-40: LGTM! Method signatures correctly refactored to accept signer parameter.All methods properly accept
ManagedSignerServiceas a parameter instead of storing it as a dependency, successfully avoiding the circular dependency issue mentioned in the PR objectives.Also applies to: 47-47, 63-85, 110-126
96-98: LGTM! Trial window and expiration logic correctly implemented.The date calculations properly determine if a wallet is within the trial window and set the appropriate expiration, aligning with the PR's auto-refill objectives for trial wallets.
apps/api/src/billing/services/refill/refill.service.spec.ts (3)
14-91: LGTM! topUpWallet tests correctly updated for signer-based flow.All test cases properly pass
managedSignerServiceas the first argument toauthorizeSpendingand verify the correct parameters, including the existing wallet, new wallet, and race condition scenarios.
127-194: LGTM! reduceWalletBalance tests correctly updated.All test cases properly pass
managedSignerServicetoauthorizeSpendingand maintain coverage of edge cases including missing wallets and addresses.
93-124: LGTM! Setup functions properly updated with ManagedSignerService.Both setup functions correctly create a
managedSignerServicemock, pass it to theRefillServiceconstructor, and return it for test assertions, following the established test patterns.Also applies to: 196-227
apps/api/src/billing/services/managed-signer/managed-signer.service.spec.ts (6)
15-15: LGTM! New imports properly support updated test dependencies.The imports for
BillingConfigService,ManagedUserWalletService, andmockConfigServiceare correctly added to support the refactored constructor dependencies.Also applies to: 17-17, 29-29
70-80: LGTM! Test correctly validates blockchain-fetched fee allowance.The test now mocks
retrieveAndCalcFeeLimitreturning 0 to verify the "Not enough funds" error, properly reflecting the shift from database values to blockchain-fetched allowances.
82-101: LGTM! Test correctly validates blockchain-fetched deployment allowance.The test now mocks
retrieveDeploymentLimitreturning 0 to verify the deployment cost error, consistent with fetching allowances from the blockchain.
595-596: LGTM! Setup function properly extended with new mock methods.The setup function correctly adds
retrieveAndCalcFeeLimitandretrieveDeploymentLimitparameters with sensible default mock implementations (1000000 and 5000000 respectively).Also applies to: 610-614
643-650: LGTM! New service mocks properly configured.The
billingConfigServiceuses themockConfigServicehelper with appropriate config values, andmanagedUserWalletServiceincludes therefillWalletFeesmethod mock.
657-672: LGTM! Constructor call updated with new dependencies.The
ManagedSignerServiceinstantiation correctly includesbillingConfigService(line 659) andmanagedUserWalletService(line 671) in the constructor call, matching the updated service signature.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/api/drizzle/0026_chief_mysterio.sql (1)
1-2: Data migration step required before applying NOT NULL constraints.As noted in the previous review, applying NOT NULL constraints without first handling existing NULL values will cause the migration to fail if any rows currently have NULL timestamps. Please add UPDATE statements to backfill NULL values before altering the column constraints.
🧹 Nitpick comments (1)
apps/api/src/billing/services/managed-signer/managed-signer.service.spec.ts (1)
610-650: Consider adding test coverage for auto-refill happy pathThe default mock for
retrieveAndCalcFeeLimitreturns 1,000,000, which is above the refill threshold of 100,000. This means the auto-refill logic (lines 179-182 inmanaged-signer.service.ts) is never exercised by the existing tests.Consider adding a test case that:
- Mocks
retrieveAndCalcFeeLimitto initially return a value belowFEE_ALLOWANCE_REFILL_THRESHOLD(e.g., 50,000)- Mocks
refillWalletFeesto succeed- Configures
retrieveAndCalcFeeLimitto return a value above the threshold on the second call (e.g., usingmockResolvedValueOnce)- Verifies
refillWalletFeeswas called with the correct arguments- Verifies the transaction proceeds successfully
💡 Example test case
it("automatically refills wallet fees when fee allowance is below threshold", async () => { const wallet = UserWalletSeeder.create({ userId: "user-123", feeAllowance: 50000, // Below threshold deploymentAllowance: 100 }); const user = UserSeeder.create({ userId: "user-123" }); const messages: EncodeObject[] = [ { typeUrl: MsgCreateLease.$type, value: MsgCreateLease.fromPartial({ bidId: { dseq: Long.fromNumber(123) } }) } ]; const { service, managedUserWalletService, balancesService } = setup({ findOneByUserId: jest.fn().mockResolvedValue(wallet), findById: jest.fn().mockResolvedValue(user), retrieveAndCalcFeeLimit: jest.fn() .mockResolvedValueOnce(50000) // Below threshold, triggers refill .mockResolvedValueOnce(150000), // After refill, above threshold signAndBroadcastWithDerivedWallet: jest.fn().mockResolvedValue({ code: 0, hash: "tx-hash", rawLog: "success" }), refreshUserWalletLimits: jest.fn().mockResolvedValue(undefined) }); await service.executeDerivedDecodedTxByUserId("user-123", messages); expect(managedUserWalletService.refillWalletFees).toHaveBeenCalledWith(service, wallet); expect(balancesService.retrieveAndCalcFeeLimit).toHaveBeenCalledTimes(2); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/api/drizzle/0026_chief_mysterio.sqlapps/api/drizzle/meta/0026_snapshot.jsonapps/api/drizzle/meta/_journal.jsonapps/api/src/billing/model-schemas/user-wallet/user-wallet.schema.tsapps/api/src/billing/services/managed-signer/managed-signer.service.spec.tsapps/api/src/billing/services/managed-signer/managed-signer.service.tsapps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.tsapps/api/src/billing/services/provider-cleanup/provider-cleanup.service.tsapps/api/src/billing/services/refill/refill.service.spec.tsapps/api/src/billing/services/refill/refill.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/deployment/services/stale-managed-deployments-cleaner/stale-managed-deployments-cleaner.service.tsapps/api/test/functional/wallets-refill.spec.ts
✅ Files skipped from review due to trivial changes (1)
- apps/api/drizzle/meta/0026_snapshot.json
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/api/src/billing/model-schemas/user-wallet/user-wallet.schema.ts
- apps/api/src/billing/services/refill/refill.service.spec.ts
- apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.ts
- apps/api/src/billing/services/provider-cleanup/provider-cleanup.service.ts
- apps/api/src/deployment/services/stale-managed-deployments-cleaner/stale-managed-deployments-cleaner.service.ts
- apps/api/drizzle/meta/_journal.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/api/src/billing/services/managed-signer/managed-signer.service.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.tsapps/api/src/billing/services/managed-signer/managed-signer.service.spec.tsapps/api/src/billing/services/refill/refill.service.tsapps/api/test/functional/wallets-refill.spec.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/billing/services/managed-signer/managed-signer.service.spec.tsapps/api/test/functional/wallets-refill.spec.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: ygrishajev
Repo: akash-network/console PR: 2274
File: apps/api/drizzle/0023_sad_adam_warlock.sql:1-1
Timestamp: 2025-11-28T09:17:28.832Z
Learning: In the Akash Network console billing system, the `user_wallets.user_id` column is required (NOT NULL). All user wallets must be associated with a user ID, and there are no anonymous wallets without a user_id in production.
📚 Learning: 2025-11-28T09:17:28.832Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2274
File: apps/api/drizzle/0023_sad_adam_warlock.sql:1-1
Timestamp: 2025-11-28T09:17:28.832Z
Learning: In the Akash Network console billing system, the `user_wallets.user_id` column is required (NOT NULL). All user wallets must be associated with a user ID, and there are no anonymous wallets without a user_id in production.
Applied to files:
apps/api/drizzle/0026_chief_mysterio.sqlapps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts
📚 Learning: 2025-10-15T16:39:55.348Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.348Z
Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.
Applied to files:
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/test/functional/wallets-refill.spec.ts
📚 Learning: 2025-11-25T17:45:44.790Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-11-25T17:45:44.790Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test
Applied to files:
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/billing/services/managed-signer/managed-signer.service.spec.ts
📚 Learning: 2025-10-31T11:26:42.138Z
Learnt from: stalniy
Repo: akash-network/console PR: 2138
File: apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts:379-382
Timestamp: 2025-10-31T11:26:42.138Z
Learning: In TypeScript/JavaScript, the pattern of checking a cached value and then performing an async operation to fetch it without proper synchronization is race condition unsafe:
```typescript
private async getAddress() {
if (!this.address) {
this.address = await this.wallet.getFirstAddress();
}
return this.address;
}
```
Multiple concurrent calls can all pass the `if (!this.address)` check before any of them sets the value, leading to duplicate async operations. This should be flagged as a race condition. Proper synchronization (mutex, atomic promise caching, or guaranteed single-threaded execution) is required.
Applied to files:
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts
📚 Learning: 2025-12-29T12:12:27.886Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2432
File: apps/api/src/billing/services/tx-manager/tx-manager.service.ts:116-118
Timestamp: 2025-12-29T12:12:27.886Z
Learning: In the Akash Console codebase (akash-network/console), v1 wallets are only used for read-only purposes such as retrieving addresses. All transaction signing operations use v2 wallets exclusively. Therefore, methods like `getDerivedWalletAddress` need to accept `WalletOptions` to support both v1 and v2 address lookups, while transaction signing methods like `signAndBroadcastWithDerivedWallet` correctly default to v2 without requiring a wallet version parameter.
Applied to files:
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.tsapps/api/test/functional/wallets-refill.spec.ts
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/api/src/billing/services/managed-signer/managed-signer.service.spec.ts
📚 Learning: 2025-12-29T13:20:43.626Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2422
File: apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts:124-128
Timestamp: 2025-12-29T13:20:43.626Z
Learning: In the wallet-settings service (apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts), auto-reload jobs are not eagerly canceled when autoReloadEnabled is disabled. Instead, jobs check the autoReloadEnabled flag at execution time and complete early if disabled. This design avoids concurrency issues that could occur with eager cancellation.
Applied to files:
apps/api/src/billing/services/managed-signer/managed-signer.service.spec.ts
🧬 Code graph analysis (3)
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts (1)
apps/api/src/billing/providers/type-registry.provider.ts (1)
TYPE_REGISTRY(34-34)
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts (4)
apps/api/src/billing/services/tx-manager/tx-manager.service.ts (1)
options(87-91)apps/api/src/billing/services/managed-signer/managed-signer.service.ts (1)
userWallet(170-186)apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts (1)
UserWalletOutput(18-22)apps/api/src/billing/services/rpc-message-service/rpc-message.service.ts (1)
SpendingAuthorizationMsgOptions(16-22)
apps/api/src/billing/services/managed-signer/managed-signer.service.spec.ts (1)
apps/api/test/mocks/config-service.mock.ts (1)
mockConfigService(16-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (16)
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts (3)
1-23: LGTM: Test imports and structureThe imports and test suite structure correctly follow the coding guidelines, using
jest-mock-extendedfor mocking and the service name reference in the describe block.
25-112: LGTM: Test cases correctly updatedAll test cases properly reflect the new method signature, passing
ManagedSignerServiceas the first argument tocreateAndAuthorizeTrialSpending. The test logic and expectations are sound.
114-164: LGTM: Setup function follows guidelinesThe setup function correctly follows the coding guidelines: positioned at the bottom of the describe block, uses inline type definition for parameters, registers all necessary dependencies including
TYPE_REGISTRYandManagedSignerService, and has no specified return type.apps/api/src/billing/services/managed-signer/managed-signer.service.ts (2)
33-48: LGTM: Constructor dependencies updatedThe constructor correctly adds
BillingConfigServiceandManagedUserWalletServicedependencies needed for the auto-refill functionality.
158-186: LGTM: Auto-refill logic correctly implementedThe validation and auto-refill logic is well-designed:
- Fetches fee allowance from the blockchain to ensure accuracy
- Conditionally refills when below threshold using
FEE_ALLOWANCE_REFILL_THRESHOLD(correctly fixed per past review)- Re-fetches allowance after refill to verify success
- Safe from infinite loops with the final assertion check
apps/api/src/billing/services/refill/refill.service.ts (1)
10-10: LGTM: Consistent refactoring to pass ManagedSignerServiceThe service correctly imports
ManagedSignerService, adds it as a constructor dependency, and consistently passes it as the first argument to allauthorizeSpendingcalls (lines 53, 74, 104).Also applies to: 24-24, 53-53, 74-74, 104-104
apps/api/test/functional/wallets-refill.spec.ts (1)
7-7: LGTM: Functional test updated for new signatureThe functional test correctly resolves
ManagedSignerServicefrom the DI container and passes it as the first argument toauthorizeSpending, aligning with the refactored method signature.Also applies to: 31-35
apps/api/src/billing/services/managed-signer/managed-signer.service.spec.ts (2)
595-650: LGTM: Test setup comprehensively updatedThe test setup correctly adds mocks for
BillingConfigServiceandManagedUserWalletService, and extendsBalancesServicewith the newretrieveAndCalcFeeLimitandretrieveDeploymentLimitmethods. The use ofmockConfigServicehelper aligns with the established pattern in the codebase.
657-672: LGTM: Constructor correctly updated with new dependenciesThe
ManagedSignerServiceinstantiation correctly passes the new dependenciesbillingConfigServiceandmanagedUserWalletServicein the proper order.apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts (7)
5-8: LGTM on imports and circular dependency handling.The type-only import for
ManagedSignerService(line 14) correctly avoids circular dependency at runtime while maintaining type safety. The date-fns utilities and http-assert are appropriate additions for the new refill logic.Also applies to: 12-12, 14-14
41-55: LGTM on signer parameter threading.The method correctly accepts and propagates the signer parameter to
authorizeSpending, maintaining the same business logic while enabling the circular dependency fix.
64-86: LGTM on signer propagation.The signer is correctly threaded through to both deployment and fee spending authorization methods.
96-111: Address guard clause correctly added - past review concern resolved.Line 97's
assert(address, 402, "Wallet is not initialized")properly guards against null/undefined addresses before usage, addressing the previous review comment about the non-null assertion risk.The trial window logic is sound:
- Calculates if wallet is within trial period by checking
createdAt > (now - TRIAL_ALLOWANCE_EXPIRATION_DAYS)- Preserves original expiration date for trial wallets via
addDays(createdAt, ...)- Non-trial or expired-trial wallets get no expiration (undefined)
113-124: LGTM on fee spending authorization.The method correctly uses the injected signer to execute funding transactions while maintaining the revoke-if-exists logic.
126-129: LGTM on deployment spending authorization.The method correctly delegates to the signer for executing the deposit deployment grant message.
88-95: JSDoc is appropriate here.The documentation clearly explains the method's purpose, the trial window logic, and parameter expectations - valuable for a method with non-obvious business rules around trial wallet handling.
This prevents transaction failures caused by out-of-sync fee allowances in the database by proactively checking and authorizing fees from the blockchain when necessary, especially for trial users.
refs #2338
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.