refactor: improves encapsulation of console-api #2249
Conversation
WalkthroughAdds REST_API_NODE_URL to env/config and threads it through DI; refactors HTTP services to use injected HttpClient and BlockHttpService composition; replaces direct axios/node-URL usage with injected services; converts several wallet internals to hard-private fields and exports WALLET_ADDRESS_PREFIX; moves DB migration/lifecycle into DI-managed factories and changes test teardowns to dispose the DI container. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
rect rgb(250,250,255)
participant App as Application Service
participant DI as tsyringe Container
participant CoreCfg as CoreConfigService
participant HttpC as CHAIN_API_HTTP_CLIENT (HttpClient)
participant Block as BlockHttpService
participant REST as REST API (akash)
end
App->>DI: resolve(Service)
App->>CoreCfg: resolve(CORE_CONFIG) -> get("REST_API_NODE_URL")
CoreCfg-->>HttpC: provide baseURL
App->>Block: call getCurrentHeight()
Block->>HttpC: get("/blocks/latest")
HttpC->>REST: HTTP GET /blocks/latest
REST-->>HttpC: 200 { block.header.height }
HttpC-->>Block: parsed height
Block-->>App: return height (number)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas to focus review on:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (79.71%) 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 #2249 +/- ##
==========================================
+ Coverage 47.19% 47.47% +0.27%
==========================================
Files 1032 1025 -7
Lines 28980 29195 +215
Branches 7567 7608 +41
==========================================
+ Hits 13677 13860 +183
- Misses 14924 14935 +11
- Partials 379 400 +21
🚀 New features to boost your workflow:
|
| REST_API_NODE_URL: z | ||
| .string() | ||
| .url() | ||
| .default(() => netConfig.getBaseAPIUrl(process.env.NETWORK || "mainnet")) |
There was a problem hiding this comment.
added default value because this doesn't work well in unit tests, probably this variable needs to be moved to a different module. But it's used by http-sdk providers in core module, so at least for now let it be here
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (14)
apps/api/src/console.ts (1)
121-123: Shutdown viacontainer.dispose()fits the DI lifecycle; optional hardeningSwitching shutdown to a single
container.dispose()is consistent with the new DI‑driven teardown and will pick up DB cleanup via theAPP_INITIALIZERdisposable. If you want to harden this path, you could wrap it in atry/catchto log dispose failures without changing behavior:-const shutdown = once(async () => { - await container.dispose(); -}); +const shutdown = once(async () => { + try { + await container.dispose(); + } catch (error) { + logger.error({ event: "SHUTDOWN_ERROR", error }); + process.exitCode = process.exitCode ?? 1; + } +});apps/api/env/.env.sandbox (1)
2-3: Env additions look good; consider key ordering for linters
REST_API_NODE_URLandDEPLOYMENT_GRANT_DENOMvalues look correct for sandbox. dotenv-linter warns about key order; if your CI enforces it, consider sorting keys alphabetically (e.g.,DEPLOYMENT_GRANT_DENOM,REST_API_NODE_URL,RPC_NODE_ENDPOINT) to avoid noise.package.json (1)
80-81: Engine/Volta bumps are reasonable; watch npm version consistencyRelaxing
"node"to^24.11.1and bumping"npm"/volta.npmto11.6.2makes sense. However,packageManageris still"[email protected]", which may cause confusion between tooling and engine expectations; consider aligningpackageManagerwith11.6.2(or keeping all three in sync) to avoid subtle version drift across environments.Also applies to: 85-85
packages/http-sdk/src/block/block-http.service.ts (1)
1-19: HttpClient-based BlockHttpService looks correct; minor parsing polish possibleThe refactor to inject an
HttpClientand useextractDatakeeps HTTP concerns encapsulated and makes the service easier to test/mocks;getCurrentHeight()returning anumberfromresponse.block.header.heightmatches expected API shape.If you want to tighten it further, you could:
- Specify radix in
parseInt(response.block.header.height, 10).- Optionally guard against unexpected shapes/values and throw a more explicit error instead of returning
NaN, so callers don’t all need to check forNaNthemselves.These are niceties rather than blockers.
apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts (1)
15-15: Delegating height lookup to BlockHttpService improves encapsulationInjecting
BlockHttpServiceand routinggetCurrentHeight()through it removes the ad-hoc HTTP call from this service and centralizes chain-height fetching, which fits the PR’s encapsulation goal. TheNumber.isNaN(height)guard is a good safety net.If you expect other kinds of invalid heights (e.g.,
<= 0), you might optionally extend the check (e.g.,!Number.isFinite(height) || height <= 0) or log more context before throwing, but current behavior is acceptable.Also applies to: 24-30, 182-187
apps/api/test/functional/dashboard-data.spec.ts (1)
3-3: DI-based teardown looks good; consider centralizing disposalUsing
await container.dispose()inafterAllmatches the DI-driven cleanup model and should properly release resources for this suite. Givensetup-functional-tests.tsalso disposes the container (with a protective try/catch), this local teardown is somewhat redundant; you may eventually centralize container disposal there to keep lifecycle concerns in one place, but it's not required for correctness. Based on learningsAlso applies to: 197-199
apps/api/test/functional/provider-graph-data.spec.ts (1)
3-3: Per-suite container.dispose is correct but overlaps with global teardownSwitching this suite to
await container.dispose()inafterAllcorrectly leverages the DI container’s disposal semantics. Sincesetup-functional-tests.tsalso disposes the container with a guardedafterAll, you’ve got overlapping cleanup; functionally fine, but you might later consolidate disposal into the shared setup to avoid scattered lifecycle logic. Based on learningsAlso applies to: 197-199
apps/api/test/functional/validators.spec.ts (1)
3-6: Switch to CORE_CONFIG-based REST node URL is aligned; consider DRYing resolutionUsing
nock(container.resolve(CORE_CONFIG).REST_API_NODE_URL)keeps the validator tests in sync with the DI-driven configuration and avoids coupling to a hard-codedapiNodeUrl, which is good. Within the samebeforeAll, you can avoid repeated resolution and slightly improve readability by caching the URL once, e.g.:beforeAll(async () => { validators = await Promise.all([ // ... ]); - nock(container.resolve(CORE_CONFIG).REST_API_NODE_URL) + const { REST_API_NODE_URL } = container.resolve(CORE_CONFIG); + + nock(REST_API_NODE_URL) .persist() .get(`/cosmos/staking/v1beta1/validators?status=BOND_STATUS_BONDED&pagination.limit=1000`) .reply(200, { /* ... */ }); - nock(container.resolve(CORE_CONFIG).REST_API_NODE_URL) + nock(REST_API_NODE_URL) .persist() .get(`/cosmos/staking/v1beta1/validators/${validators[0].operatorAddress}`) .reply(200, { /* ... */ }); - nock(container.resolve(CORE_CONFIG).REST_API_NODE_URL).persist().get( - `/cosmos/staking/v1beta1/validators/${validators[1].operatorAddress}` - ).reply(404); + nock(REST_API_NODE_URL) + .persist() + .get(`/cosmos/staking/v1beta1/validators/${validators[1].operatorAddress}`) + .reply(404); });Not mandatory, but keeps the test intent clearer and the base URL consistent.
Also applies to: 26-160
apps/api/test/functional/graph-data.spec.ts (1)
3-3: Graph Data suite teardown uses DI correctly; optional to rely on shared setupUsing
await container.dispose()in this suite’safterAllcorrectly hooks into the DI container’s disposal mechanism. Since the functional test setup also disposes the container with error-tolerant logic, this is somewhat redundant; you can leave it as-is or, in a future cleanup PR, centralize disposal insetup-functional-tests.tsto reduce duplication. Based on learningsAlso applies to: 169-171
apps/api/test/functional/sign-and-broadcast-tx.spec.ts (1)
9-9: REST API node URL mocking now follows CORE_CONFIG; minor DRY opportunitySwitching the REST nock in
blockNodetocontainer.resolve(CORE_CONFIG).REST_API_NODE_URLkeeps this test aligned with the DI configuration and avoids stale constants. If you want to tighten it up, you could resolve once per call:function blockNode(code: string, message: string) { const RPC_NODE_ENDPOINT = container.resolve(BillingConfigService).get("RPC_NODE_ENDPOINT"); + const { REST_API_NODE_URL } = container.resolve(CORE_CONFIG); - nock(container.resolve(CORE_CONFIG).REST_API_NODE_URL) + nock(REST_API_NODE_URL) .persist() .get(/.*/) .replyWithError({ code, message }) .post(/.*/) .replyWithError({ code, message });Behavior is unchanged; this just makes the target URL more explicit.
Also applies to: 122-127
apps/api/test/functional/provider-earnings.spec.ts (1)
5-5: Provider Earnings teardown aligns with DI cleanup; duplication is acceptableDisposing the DI container in this suite’s
afterAllbefore clearingmcachematches the new resource-management approach and should ensure DB/HTTP-resources are released. As with other functional specs, this overlaps with the container disposal insetup-functional-tests.ts, but thanks to the guarded global call it’s safe; you can optionally centralize disposal later if you want a single teardown point. Based on learningsAlso applies to: 37-40
apps/api/test/functional/bids.spec.ts (1)
50-50: Consider adding container disposal in afterAll.The file now uses the DI container but doesn't dispose of it in an
afterAllhook. Other test files in this PR (e.g., deployments.spec.ts, addresses.spec.ts) addawait container.dispose()inafterAllto ensure proper cleanup.Add the following after line 31:
+ afterAll(async () => { + await container.dispose(); + }); + async function setup() {apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts (1)
54-60: Derived wallet flow increateWalletlooks solid; consider richer loggingThe new flow cleanly delegates derivation to the injected master wallet and returns just the address, which improves encapsulation and keeps this service mnemonic‑agnostic. As a minor improvement, you might also log the
addressIndexalongside the address in Line 57 to make debugging derived-wallet issues easier in multi-index scenarios.apps/api/src/core/providers/postgres.provider.ts (1)
20-28: Postgres client/DB DI wiring is well-structured; optional: add migration loggingThe new
APP_PG_CLIENTtoken,getPgDatabasefactory, andPOSTGRES_DBregistration cleanly centralize connection management and keep drizzle configuration (schema + logger) in one place. Using a dedicated client insidemigratePGwith afinally { await migrationClient.end(); }block is a good separation of concerns, and theDB_HEALTHCHECKfactory reusingAPP_PG_CLIENTkeeps health checks lightweight.As an optional improvement, you might add explicit log messages before and after
migrate(pgMigrationDatabase, …)so operational logs clearly show when migrations start and complete.Also applies to: 31-34, 36-46, 48-50, 57-64, 68-70, 79-79
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (42)
apps/api/env/.env.mainnet(1 hunks)apps/api/env/.env.sandbox(1 hunks)apps/api/src/billing/lib/wallet/wallet.ts(1 hunks)apps/api/src/billing/services/financial-stats/financial-stats.service.ts(2 hunks)apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts(2 hunks)apps/api/src/chain/services/block-http/block-http.service.spec.ts(1 hunks)apps/api/src/console.ts(1 hunks)apps/api/src/core/config/env.config.ts(3 hunks)apps/api/src/core/config/index.ts(0 hunks)apps/api/src/core/providers/config.provider.ts(1 hunks)apps/api/src/core/providers/http-sdk.provider.ts(1 hunks)apps/api/src/core/providers/postgres.provider.ts(2 hunks)apps/api/src/core/services/feature-flags/feature-flags.service.spec.ts(2 hunks)apps/api/src/db/dbConnection.ts(1 hunks)apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts(3 hunks)apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.ts(0 hunks)apps/api/src/utils/constants.ts(0 hunks)apps/api/test/functional/addresses.spec.ts(9 hunks)apps/api/test/functional/balances.spec.ts(4 hunks)apps/api/test/functional/bids.spec.ts(2 hunks)apps/api/test/functional/create-deployment.spec.ts(3 hunks)apps/api/test/functional/dashboard-data.spec.ts(2 hunks)apps/api/test/functional/deployments.spec.ts(8 hunks)apps/api/test/functional/gpu.spec.ts(2 hunks)apps/api/test/functional/graph-data.spec.ts(2 hunks)apps/api/test/functional/managed-api-deployment-flow.spec.ts(2 hunks)apps/api/test/functional/network-capacity.spec.ts(2 hunks)apps/api/test/functional/nodes-v1.spec.ts(2 hunks)apps/api/test/functional/proposals.spec.ts(5 hunks)apps/api/test/functional/provider-deployments.spec.ts(2 hunks)apps/api/test/functional/provider-earnings.spec.ts(2 hunks)apps/api/test/functional/provider-graph-data.spec.ts(2 hunks)apps/api/test/functional/providers.spec.ts(2 hunks)apps/api/test/functional/sign-and-broadcast-tx.spec.ts(2 hunks)apps/api/test/functional/validators.spec.ts(4 hunks)apps/api/test/services/test-database.service.ts(1 hunks)apps/api/test/services/test-wallet.service.ts(3 hunks)apps/api/test/setup-functional-tests.ts(2 hunks)apps/api/test/setup-unit-tests.ts(1 hunks)apps/provider-proxy/package.json(1 hunks)package.json(1 hunks)packages/http-sdk/src/block/block-http.service.ts(2 hunks)
💤 Files with no reviewable changes (3)
- apps/api/src/core/config/index.ts
- apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.ts
- apps/api/src/utils/constants.ts
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: baktun14
Repo: akash-network/console PR: 1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: The tsyringe container in the Akash Network console project has been extended with a custom dispose() method that iterates over all registered instances and calls dispose() on each one that implements it, enabling proper resource cleanup during shutdown.
Applied to files:
apps/api/test/setup-functional-tests.tsapps/api/test/functional/graph-data.spec.tsapps/api/test/functional/gpu.spec.tsapps/api/test/setup-unit-tests.tsapps/api/test/functional/network-capacity.spec.tsapps/api/test/functional/providers.spec.tsapps/api/test/functional/nodes-v1.spec.tsapps/api/test/functional/dashboard-data.spec.tsapps/api/test/functional/provider-deployments.spec.tsapps/api/test/functional/provider-graph-data.spec.ts
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: In the Akash Network console project, services like JobQueueService implement dispose() methods for resource cleanup, and the tsyringe container has been extended with a dispose() method that iterates over all registered instances and calls their dispose() methods, enabling proper shutdown orchestration.
Applied to files:
apps/api/test/setup-functional-tests.tsapps/api/test/functional/graph-data.spec.tsapps/api/test/setup-unit-tests.tsapps/api/test/functional/network-capacity.spec.tsapps/api/src/console.tsapps/api/test/functional/providers.spec.tsapps/api/test/functional/nodes-v1.spec.tsapps/api/src/core/providers/http-sdk.provider.tsapps/api/test/functional/dashboard-data.spec.tsapps/api/test/functional/provider-deployments.spec.tsapps/api/test/functional/provider-graph-data.spec.ts
📚 Learning: 2025-06-08T03:07:13.871Z
Learnt from: stalniy
Repo: akash-network/console PR: 1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Applied to files:
apps/api/test/functional/providers.spec.tsapps/api/test/functional/provider-deployments.spec.ts
📚 Learning: 2025-11-12T09:03:40.132Z
Learnt from: stalniy
Repo: akash-network/console PR: 0
File: :0-0
Timestamp: 2025-11-12T09:03:40.132Z
Learning: For backend services (like the Indexer), prefer using createOtelLogger from "akashnetwork/logging/otel" to include OpenTelemetry trace context in logs.
Applied to files:
apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts
📚 Learning: 2025-07-09T03:30:51.075Z
Learnt from: devalpatel67
Repo: akash-network/console PR: 1646
File: apps/api/src/provider/services/provider-earnings/provider-earnings.service.ts:41-41
Timestamp: 2025-07-09T03:30:51.075Z
Learning: The `getProviderEarningsAtHeight` function in `apps/api/src/services/db/statsService.ts` is designed to be flexible and can be used for different earnings calculations: (1) For total lifetime earnings, pass `provider.createdHeight` as the second parameter, (2) For date-range earnings, pass `fromBlock.height` as the second parameter. The second parameter acts as a lower bound and the third parameter as an upper bound for the earnings calculation.
Applied to files:
apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.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/test/functional/create-deployment.spec.tsapps/api/test/functional/deployments.spec.tsapps/api/test/services/test-wallet.service.tsapps/api/src/billing/lib/wallet/wallet.tsapps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts
📚 Learning: 2025-08-12T13:52:38.708Z
Learnt from: stalniy
Repo: akash-network/console PR: 1800
File: apps/deploy-web/next.config.js:163-165
Timestamp: 2025-08-12T13:52:38.708Z
Learning: In the Akash Console project, akashnetwork/env-loader is used at the top of next.config.js files to automatically load environment variables from env/.env files into process.env. SENTRY_ORG and SENTRY_PROJECT are stored as public configuration values in apps/deploy-web/env/.env and are loaded this way, while only SENTRY_AUTH_TOKEN is handled as a GitHub secret in workflows.
Applied to files:
apps/api/env/.env.mainnetapps/api/src/core/config/env.config.ts
📚 Learning: 2025-08-12T13:52:38.708Z
Learnt from: stalniy
Repo: akash-network/console PR: 1800
File: apps/deploy-web/next.config.js:163-165
Timestamp: 2025-08-12T13:52:38.708Z
Learning: In the Akash Console project, SENTRY_ORG and SENTRY_PROJECT are stored as public configuration values in apps/deploy-web/env/.env file and loaded by akashnetwork/env-loader, not as GitHub secrets. Only SENTRY_AUTH_TOKEN should be handled as a secret.
Applied to files:
apps/api/env/.env.mainnet
📚 Learning: 2025-05-28T20:42:58.200Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 1372
File: apps/api/src/dashboard/services/stats/stats.service.ts:0-0
Timestamp: 2025-05-28T20:42:58.200Z
Learning: The user successfully implemented CosmosHttpService with retry logic using axiosRetry, exponential backoff, and proper error handling for Cosmos API calls, replacing direct axios usage in StatsService.
Applied to files:
apps/api/src/billing/services/financial-stats/financial-stats.service.ts
📚 Learning: 2025-06-10T01:25:37.604Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 1458
File: apps/api/src/network/services/network/network.service.ts:0-0
Timestamp: 2025-06-10T01:25:37.604Z
Learning: In the Akash Network Console codebase, network parameters are validated at the schema level using Zod schemas before reaching service methods, making default cases in switch statements unnecessary for validated enum-like parameters.
Applied to files:
apps/api/src/core/config/env.config.ts
📚 Learning: 2025-06-10T01:25:36.097Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 1458
File: apps/api/src/network/services/network/network.service.ts:0-0
Timestamp: 2025-06-10T01:25:36.097Z
Learning: In the Akash Console API, network parameter validation is handled at the HTTP schema level using Zod validation, making additional runtime validation checks in service layers unnecessary. The validation-at-the-boundary approach is preferred over defensive programming throughout every layer.
Applied to files:
apps/api/src/core/config/env.config.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/lib/wallet/wallet.ts
🧬 Code graph analysis (18)
apps/api/test/functional/proposals.spec.ts (1)
apps/api/src/core/providers/config.provider.ts (1)
CORE_CONFIG(7-7)
apps/api/test/functional/validators.spec.ts (1)
apps/api/src/core/providers/config.provider.ts (1)
CORE_CONFIG(7-7)
apps/api/test/functional/bids.spec.ts (1)
apps/api/src/core/providers/config.provider.ts (1)
CORE_CONFIG(7-7)
apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts (1)
packages/http-sdk/src/block/block-http.service.ts (1)
BlockHttpService(12-20)
apps/api/test/functional/addresses.spec.ts (1)
apps/api/src/core/providers/config.provider.ts (1)
CORE_CONFIG(7-7)
apps/api/test/functional/create-deployment.spec.ts (2)
apps/api/src/core/providers/config.provider.ts (1)
CORE_CONFIG(7-7)packages/http-sdk/src/block/block-http.service.ts (1)
BlockHttpService(12-20)
apps/api/test/functional/deployments.spec.ts (3)
apps/api/src/core/providers/config.provider.ts (1)
CORE_CONFIG(7-7)apps/api/src/utils/constants.ts (2)
deploymentVersion(18-18)marketVersion(19-19)apps/api/test/seeders/lease-api-response.seeder.ts (1)
LeaseApiResponseSeeder(71-145)
apps/api/src/core/providers/http-sdk.provider.ts (5)
packages/http-sdk/src/utils/httpClient.ts (2)
HttpClient(23-23)createHttpClient(5-21)packages/http-sdk/src/balance/balance-http.service.ts (1)
BalanceHttpService(20-29)packages/http-sdk/src/bid/bid-http.service.ts (1)
BidHttpService(102-112)packages/http-sdk/src/provider/provider-http.service.ts (1)
ProviderHttpService(6-42)packages/http-sdk/src/block/block-http.service.ts (1)
BlockHttpService(12-20)
apps/api/test/functional/balances.spec.ts (1)
apps/api/src/core/providers/config.provider.ts (1)
CORE_CONFIG(7-7)
apps/api/src/core/services/feature-flags/feature-flags.service.spec.ts (2)
apps/api/src/core/config/env.config.ts (1)
CoreConfig(42-42)apps/api/src/core/providers/config.provider.ts (1)
CoreConfig(13-13)
apps/api/test/functional/managed-api-deployment-flow.spec.ts (1)
apps/api/src/core/providers/config.provider.ts (1)
CORE_CONFIG(7-7)
apps/api/src/core/providers/postgres.provider.ts (4)
apps/api/test/services/test-database.service.ts (1)
postgres(15-18)apps/api/src/core/providers/config.provider.ts (2)
CORE_CONFIG(7-7)CoreConfig(13-13)apps/api/src/core/config/env.config.ts (1)
CoreConfig(42-42)apps/api/src/core/services/postgres-logger/postgres-logger.service.ts (1)
PostgresLoggerService(11-38)
apps/api/src/core/providers/config.provider.ts (1)
apps/api/src/core/config/env.config.ts (2)
CoreConfig(42-42)envSchema(4-40)
apps/api/test/functional/sign-and-broadcast-tx.spec.ts (1)
apps/api/src/core/providers/config.provider.ts (1)
CORE_CONFIG(7-7)
apps/api/src/billing/services/financial-stats/financial-stats.service.ts (4)
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts (1)
singleton(27-135)packages/http-sdk/src/cosmos/cosmos-http.service.ts (1)
CosmosHttpService(21-109)apps/api/src/billing/providers/wallet.provider.ts (1)
MANAGED_MASTER_WALLET(9-9)apps/api/src/billing/lib/wallet/wallet.ts (1)
Wallet(9-62)
apps/api/src/core/config/env.config.ts (3)
packages/net/src/index.ts (1)
netConfig(3-3)apps/api/src/core/providers/config.provider.ts (1)
CoreConfig(13-13)apps/api/src/billing/config/env.config.ts (1)
envSchema(5-31)
apps/api/src/chain/services/block-http/block-http.service.spec.ts (1)
packages/http-sdk/src/block/block-http.service.ts (1)
BlockHttpService(12-20)
apps/api/test/services/test-wallet.service.ts (1)
apps/api/src/billing/lib/wallet/wallet.ts (2)
WALLET_ADDRESS_PREFIX(7-7)Wallet(9-62)
🪛 dotenv-linter (4.0.0)
apps/api/env/.env.mainnet
[warning] 2-2: [UnorderedKey] The REST_API_NODE_URL key should go before the RPC_NODE_ENDPOINT key
(UnorderedKey)
apps/api/env/.env.sandbox
[warning] 2-2: [UnorderedKey] The REST_API_NODE_URL key should go before the RPC_NODE_ENDPOINT key
(UnorderedKey)
[warning] 3-3: [UnorderedKey] The DEPLOYMENT_GRANT_DENOM key should go before the REST_API_NODE_URL key
(UnorderedKey)
⏰ 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). (20)
- GitHub Check: validate (apps/api) / validate-unsafe
- GitHub Check: validate (apps/log-collector) / validate-unsafe
- GitHub Check: validate (apps/provider-console) / validate-unsafe
- GitHub Check: validate (apps/indexer) / validate-unsafe
- GitHub Check: validate (packages) / validate-unsafe
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (29)
apps/api/src/db/dbConnection.ts (1)
55-69: DB teardown viadispose()integrates cleanly with container-driven shutdownWiring
chainDb.close()anduserDb.close()into theAPP_INITIALIZER'sdispose()fits the DisposableRegistry +container.dispose()pattern, so DB pools will now be closed as part of the global DI shutdown instead of ad‑hoc calls. This aligns well with the shutdown changes inconsole.ts. Verification confirms that the previously exportedcloseConnectionshelper has no remaining references in the codebase, so the cleanup is complete and safe.apps/provider-proxy/package.json (1)
64-64: ts-node devDependency matches existing dev:inspect usageAdding
ts-nodehere aligns with thedev:inspectscript (-r ts-node/register) and prevents runtime "module not found" issues during local debugging. Just ensure^10.9.2remains compatible with your Node/TypeScript targets; bump if future tooling requires it.apps/api/src/core/services/feature-flags/feature-flags.service.spec.ts (1)
5-5: Spec now correctly typed against CoreConfigSwitching the import to
CoreConfigand updatingconfig?: Partial<CoreConfig>keeps this spec aligned with the central env schema and avoids drift from the real config type while still allowing partial overrides in tests. No issues from a typing or behavior standpoint.Also applies to: 167-173
apps/api/src/chain/services/block-http/block-http.service.spec.ts (1)
3-7: Local setup with mocked BlockHttpServiceCommon keeps delegation test cleanUsing a type-only import for
BlockHttpServiceCommonplusmock<BlockHttpServiceCommon>()in asetup()helper is a nice improvement: the test now clearly verifies that the APIBlockHttpServicedelegates to the shared HTTP-SDK service without leaking implementation details, and the mock wiring is straightforward.Also applies to: 11-23
apps/api/test/functional/providers.spec.ts (1)
7-7: Functional test teardown now aligns with DI container lifecycleImporting the shared
containerand callingawait container.dispose()inafterAllensures DI-managed resources (DB connections, background services, etc.) are cleaned up consistently across tests, withmcache.clear()handling the remaining in-memory cache. This matches the project’s established shutdown pattern.Also applies to: 79-82
apps/api/test/setup-functional-tests.ts (1)
3-4: Container-based teardown pattern is consistent and defensiveThe global
afterAllthat first (best-effort) callsawait container.dispose()and then tears down the test DB and cache fits the customcontainer.dispose()contract and safely tolerates suites that might already have disposed the container. No issues from a lifecycle perspective. Based on learningsAlso applies to: 41-45
apps/api/test/setup-unit-tests.ts (1)
3-11: Unit-test DI teardown is straightforward and consistentAdding an
afterAllthat best-effort disposes the tsyringe container is a clean way to ensure unit-test DI resources are released without impacting suites that might callcontainer.dispose()themselves. No changes needed. Based on learningsapps/api/env/.env.mainnet (1)
2-2: LGTM! REST_API_NODE_URL addition supports DI-based configuration.The new environment variable aligns with the broader refactoring to provide configuration through the CORE_CONFIG injection token. The static analysis hint about key ordering is pedantic and can be safely ignored—environment variables don't require alphabetical ordering.
apps/api/test/functional/nodes-v1.spec.ts (1)
4-4: LGTM! DI container disposal replaces direct connection cleanup.The migration from
closeConnections()tocontainer.dispose()aligns with the established pattern where the DI container orchestrates cleanup of all registered resources.Based on learnings.
Also applies to: 19-19
apps/api/test/functional/network-capacity.spec.ts (1)
3-3: LGTM! Consistent DI container lifecycle management.The test teardown now uses
container.dispose()for resource cleanup, consistent with the broader DI lifecycle pattern.Based on learnings.
Also applies to: 196-196
apps/api/test/functional/gpu.spec.ts (1)
13-13: LGTM! DI container disposal pattern applied consistently.The test teardown migrates to
container.dispose(), maintaining consistency with the established resource cleanup pattern.Based on learnings.
Also applies to: 37-37
apps/api/test/functional/balances.spec.ts (1)
8-8: LGTM! Configuration now sourced from DI container.The migration from the static
apiNodeUrlconstant tocontainer.resolve(CORE_CONFIG).REST_API_NODE_URLimproves encapsulation by using the DI-provided configuration. The pattern is applied consistently across all three nock setups.Also applies to: 199-223
apps/api/test/functional/provider-deployments.spec.ts (1)
5-5: LGTM! Test teardown uses DI container disposal.The change to
container.dispose()aligns with the established DI lifecycle pattern used consistently across the test suite.Based on learnings.
Also applies to: 114-114
apps/api/test/functional/proposals.spec.ts (1)
2-4: LGTM! Configuration migration improves encapsulation.The migration from static
apiNodeUrltocontainer.resolve(CORE_CONFIG).REST_API_NODE_URLis applied consistently across all four nock setups. This change aligns with the PR's goal of improving encapsulation through DI-based configuration.Also applies to: 99-276
apps/api/test/functional/managed-api-deployment-flow.spec.ts (1)
23-23: LGTM! Network outage simulation uses DI-provided configuration.The
blockNode()function now sources the REST API URL fromcontainer.resolve(CORE_CONFIG).REST_API_NODE_URL, maintaining consistency with the broader configuration migration pattern.Also applies to: 346-346
apps/api/test/services/test-database.service.ts (1)
47-51: LGTM!The try/finally block ensures the migration client is properly closed regardless of success or failure, preventing connection leaks.
apps/api/test/functional/deployments.spec.ts (2)
108-108: LGTM!Proper DI container disposal ensures clean test teardown and prevents resource leaks across test runs.
138-200: LGTM!Consistent migration from hardcoded
apiNodeUrlto DI-basedREST_API_NODE_URLacross all nock setups improves configurability and testability.apps/api/src/core/providers/http-sdk.provider.ts (2)
24-30: LGTM!Factory-based registration ensures the REST_API_NODE_URL is resolved from CoreConfigService at runtime, improving testability and configuration flexibility.
32-45: LGTM!Correctly categorizes services based on their constructor signatures:
- SERVICES: Extend HttpService, accept
baseURLconfig- NON_AXIOS_SERVICES: Accept HttpClient instance directly
The migration of BlockHttpService to NON_AXIOS_SERVICES aligns with its constructor signature.
apps/api/test/functional/addresses.spec.ts (2)
42-42: LGTM!Proper container disposal in
afterAllensures clean test teardown.
227-489: LGTM!Comprehensive migration from hardcoded
apiNodeUrlto DI-basedCORE_CONFIG.REST_API_NODE_URLacross all HTTP mock setups improves test configurability.apps/api/test/functional/create-deployment.spec.ts (2)
74-74: LGTM!Using
BlockHttpService.getCurrentHeight()eliminates code duplication and aligns with the DI-based service pattern.
39-56: LGTM!Migration to
CORE_CONFIG.REST_API_NODE_URLfor nock setup is consistent with other test files in this PR.apps/api/test/services/test-wallet.service.ts (1)
65-75: LGTM!The refactored wallet creation flow properly separates mnemonic generation from Wallet construction, aligning with the new Wallet API that accepts a mnemonic string.
apps/api/src/core/config/env.config.ts (1)
28-31: Verify the default value computation timing: VERIFIED - NO ISSUES FOUNDThe implementation is correct. The default value is properly evaluated after environment variables are loaded:
Setup Order Confirmed: Jest configuration shows
setupFiles(which loads environment variables viadotenv) executes beforesetupFilesAfterEnv(which creates the container). This ensuresprocess.env.NETWORKis available when the default function is evaluated.Tests Get Expected Default: Tests without an explicit
NETWORKenvironment variable correctly fall back to"mainnet"via theprocess.env.NETWORK || "mainnet"expression, andnetConfig.getBaseAPIUrl("mainnet")resolves to a valid API endpoint:"https://rest-akash.ecostake.com".Runtime Behavior Verified: All 30+ test files successfully resolve
REST_API_NODE_URLfrom the container at runtime, confirming the pattern works correctly across the test suite.The code works as designed.
apps/api/src/core/providers/config.provider.ts (1)
1-13: Config DI refactor is clean and type-safeSwitching
CORE_CONFIGto a typed DI token with a cachedenvSchema.parse(process.env)factory removes the global mutable config object while keeping validation and strong typing. This aligns well with the PR’s encapsulation goal.apps/api/src/billing/services/financial-stats/financial-stats.service.ts (1)
2-4: Encapsulation of wallet and Cosmos HTTP access looks goodInjecting the managed master wallet and the HTTP-based Cosmos client into this service removes direct mnemonic/config concerns from stats logic and makes both balance and community pool lookups rely on shared abstractions. The behavior (USDC selection and
"0"fallback) remains consistent with the prior implementation.Also applies to: 7-8, 16-18, 24-27, 51-53
apps/api/src/billing/lib/wallet/wallet.ts (1)
7-8: Wallet internals and derived-wallet support are correctly encapsulatedCentralizing the Bech32 prefix via
WALLET_ADDRESS_PREFIX, moving internals to#private fields, and addingcreateDerivedWalletgive you a clean, focused abstraction for both root and derived wallets. ThesignAminoimplementation’sthis.#aminoSignerPromise ??=pattern safely caches the promise itself, which avoids the async race condition seen in “check-then-await” patterns, whilecreateDerivedWalletcorrectly reuses the same mnemonic with an index-specific HD path.Also applies to: 14-16, 19-24, 29-35, 39-40, 43-44, 47-50, 53-56, 58-61
| const hdWallet = await DirectSecp256k1HdWallet.generate(24, { prefix: WALLET_ADDRESS_PREFIX }); | ||
| const wallet = new Wallet(hdWallet.mnemonic); |
There was a problem hiding this comment.
generate mnemonic separately, so then we can get rid of getMnemonic method on Wallet class
stalniy
force-pushed
the
refactor/master-wallet
branch
from
3e5a4fa to
f279e1f
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
stalniy
force-pushed
the
refactor/master-wallet
branch
from
f279e1f to
98e68d1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (11)
apps/api/env/.env.sandbox (1)
1-3: Consider ordering environment variables alphabetically.The dotenv-linter flags unordered keys. While functionally inconsequential, sorting alphabetically improves consistency and readability of the env file.
Suggested ordering:
-RPC_NODE_ENDPOINT=https://rpc.sandbox-2.aksh.pw:443 -REST_API_NODE_URL=https://api.sandbox-2.aksh.pw:443 -DEPLOYMENT_GRANT_DENOM=uakt +DEPLOYMENT_GRANT_DENOM=uakt +REST_API_NODE_URL=https://api.sandbox-2.aksh.pw:443 +RPC_NODE_ENDPOINT=https://rpc.sandbox-2.aksh.pw:443packages/http-sdk/src/block/block-http.service.ts (1)
15-18: Consider adding radix toparseIntfor explicit base-10 parsing.While modern JavaScript handles this correctly for decimal strings, explicitly specifying radix 10 is a best practice that improves code clarity and prevents potential edge cases.
- return parseInt(response.block.header.height); + return parseInt(response.block.header.height, 10);apps/api/src/billing/lib/wallet/wallet.ts (1)
46-56: Lazy Amino signer creation pattern is safe and efficient; consider an optional guardThe
signAminoimplementation:
- Reuses the already‑constructed direct wallet.
- Lazily creates a single
Secp256k1HdWalletinstance viathis.#aminoSignerPromise ??= …and then awaits that promise.Because the
??=assignment happens synchronously afterawait this.#instanceAsPromised, concurrent calls will share the same#aminoSignerPromiseinstead of racing to create multiple Amino wallets, avoiding the unsafe “check then await” pattern called out in previous learnings. Based on learnings, this is a good, race‑free caching approach.For
getFirstAddress, assuming at least one account is fine for Cosmos HD wallets; if you ever expect custom options that could yield zero accounts, you might optionally add an explicit check and throw a clearer error instead of relying onaccounts[0].apps/api/test/functional/provider-earnings.spec.ts (1)
70-70: Avoid casting toany.The coding guidelines prohibit using
anytype. Define a proper error response type.- const data = (await res.json()) as any; + const data = (await res.json()) as { message: string };apps/api/src/core/services/feature-flags/feature-flags.service.spec.ts (2)
183-188: Avoid usingRecord<string, any>.Per coding guidelines, avoid using
any. Consider typing the config mock return value properly.({ FEATURE_FLAGS_ENABLE_ALL: false, UNLEASH_SERVER_API_URL: "http://localhost:4242/api", UNLEASH_SERVER_API_TOKEN: "default:development", DEPLOYMENT_ENV: "development", NODE_ENV: "development", NETWORK: "mainnet", ...input.config - }) as Record<string, any> + }) as Partial<CoreConfig> )[key]
209-211: Avoid casting toany.Per coding guidelines, avoid using
any. Define a proper type for the execution context mock.apps/api/test/functional/providers.spec.ts (1)
101-101: Avoid casting toanyin response handling.Per coding guidelines, avoid using
any. Consider defining a proper type for provider API responses.+type ProviderResponse = { + owner: string; + // add other relevant fields +}; - const data = (await response.json()) as any; + const data = (await response.json()) as ProviderResponse[];This would improve type safety across multiple test assertions.
Also applies to: 110-110, 119-119, 130-130
apps/api/env/.env.mainnet (1)
2-2: Consider reordering env keys to satisfy dotenv-linter
dotenv-linterwarns thatREST_API_NODE_URLshould come beforeRPC_NODE_ENDPOINT. Reordering the lines will keep the linter quiet without changing behavior.apps/api/src/console.ts (1)
107-109: Confirm that all DB/chain resources used by the CLI are covered bycontainer.dispose()
shutdownnow only awaitscontainer.dispose(), whileexecuteCliHandlerstill callschainDb.authenticate(). Given the customcontainer.dispose()is intended to clean up disposable services, please double‑check that:
- The connections created via
chainDb.authenticate()(and any other DB pools) are closed as part of container‑managed disposal, and- CLI commands terminate cleanly without hanging on open DB handles.
If any DB connection is still created outside DI, it may need either a dedicated disposable service or an explicit close in
shutdown.apps/api/test/functional/addresses.spec.ts (1)
6-6: Functional address tests correctly switched to DI‑driven REST API URL and container disposal
- Using
container.resolve(CORE_CONFIG).REST_API_NODE_URLas the nock base keeps tests aligned with runtime configuration and avoids hardcoded API URLs.afterAllnow relying oncontainer.dispose()fits the shared test teardown strategy.As a minor improvement, you could resolve
CORE_CONFIGonce insidesetupand storeconst { REST_API_NODE_URL } = container.resolve(CORE_CONFIG);to avoid repeatingcontainer.resolve(...)in every nock call, but this is purely for readability.Also applies to: 9-10, 13-13, 41-45, 227-227, 243-243, 266-266, 296-296, 319-319, 381-381, 489-489
apps/api/src/core/providers/http-sdk.provider.ts (1)
32-37: Service factories correctly pull REST_API_NODE_URL; consider minor duplication reductionRegistering
BalanceHttpService,BidHttpService, andProviderHttpServicevia factories that readREST_API_NODE_URLfromCoreConfigServicekeeps these services consistent withCHAIN_API_HTTP_CLIENTand the new config flow. If this pattern grows, you might later want a small helper (e.g.,getChainApiBaseUrl(container)) to avoid repeating theCoreConfigServiceresolution and string literal, tracked in a separate issue to keep this PR focused, per project preference. Based on learnings, this cross-cutting refactor is better as a follow-up.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (41)
apps/api/env/.env.mainnet(1 hunks)apps/api/env/.env.sandbox(1 hunks)apps/api/src/billing/lib/wallet/wallet.ts(1 hunks)apps/api/src/billing/services/financial-stats/financial-stats.service.ts(2 hunks)apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts(0 hunks)apps/api/src/chain/services/block-http/block-http.service.spec.ts(1 hunks)apps/api/src/console.ts(1 hunks)apps/api/src/core/config/env.config.ts(3 hunks)apps/api/src/core/config/index.ts(0 hunks)apps/api/src/core/providers/config.provider.ts(1 hunks)apps/api/src/core/providers/http-sdk.provider.ts(1 hunks)apps/api/src/core/providers/postgres.provider.ts(2 hunks)apps/api/src/core/services/feature-flags/feature-flags.service.spec.ts(2 hunks)apps/api/src/db/dbConnection.ts(1 hunks)apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts(3 hunks)apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.ts(0 hunks)apps/api/src/utils/constants.ts(0 hunks)apps/api/test/functional/addresses.spec.ts(9 hunks)apps/api/test/functional/balances.spec.ts(4 hunks)apps/api/test/functional/bids.spec.ts(2 hunks)apps/api/test/functional/create-deployment.spec.ts(3 hunks)apps/api/test/functional/dashboard-data.spec.ts(2 hunks)apps/api/test/functional/deployments.spec.ts(8 hunks)apps/api/test/functional/gpu.spec.ts(2 hunks)apps/api/test/functional/graph-data.spec.ts(2 hunks)apps/api/test/functional/managed-api-deployment-flow.spec.ts(2 hunks)apps/api/test/functional/network-capacity.spec.ts(2 hunks)apps/api/test/functional/nodes-v1.spec.ts(2 hunks)apps/api/test/functional/proposals.spec.ts(5 hunks)apps/api/test/functional/provider-deployments.spec.ts(2 hunks)apps/api/test/functional/provider-earnings.spec.ts(2 hunks)apps/api/test/functional/provider-graph-data.spec.ts(2 hunks)apps/api/test/functional/providers.spec.ts(2 hunks)apps/api/test/functional/validators.spec.ts(4 hunks)apps/api/test/services/test-database.service.ts(1 hunks)apps/api/test/services/test-wallet.service.ts(3 hunks)apps/api/test/setup-functional-tests.ts(2 hunks)apps/api/test/setup-unit-tests.ts(1 hunks)apps/provider-proxy/package.json(1 hunks)package.json(1 hunks)packages/http-sdk/src/block/block-http.service.ts(2 hunks)
💤 Files with no reviewable changes (4)
- apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.ts
- apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts
- apps/api/src/core/config/index.ts
- apps/api/src/utils/constants.ts
🚧 Files skipped from review as they are similar to previous changes (19)
- apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts
- apps/api/test/setup-unit-tests.ts
- apps/api/test/functional/bids.spec.ts
- apps/api/test/functional/gpu.spec.ts
- apps/api/test/functional/graph-data.spec.ts
- apps/api/test/functional/balances.spec.ts
- apps/api/test/functional/deployments.spec.ts
- apps/provider-proxy/package.json
- apps/api/test/setup-functional-tests.ts
- apps/api/test/functional/provider-graph-data.spec.ts
- apps/api/test/functional/validators.spec.ts
- apps/api/test/functional/network-capacity.spec.ts
- apps/api/test/functional/managed-api-deployment-flow.spec.ts
- apps/api/test/functional/proposals.spec.ts
- apps/api/src/chain/services/block-http/block-http.service.spec.ts
- apps/api/test/services/test-database.service.ts
- apps/api/test/services/test-wallet.service.ts
- package.json
- apps/api/test/functional/create-deployment.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/test/functional/provider-deployments.spec.tsapps/api/src/core/services/feature-flags/feature-flags.service.spec.tsapps/api/test/functional/nodes-v1.spec.tsapps/api/src/core/providers/config.provider.tsapps/api/test/functional/providers.spec.tspackages/http-sdk/src/block/block-http.service.tsapps/api/src/console.tsapps/api/test/functional/addresses.spec.tsapps/api/src/billing/services/financial-stats/financial-stats.service.tsapps/api/src/db/dbConnection.tsapps/api/src/core/providers/http-sdk.provider.tsapps/api/src/core/config/env.config.tsapps/api/test/functional/dashboard-data.spec.tsapps/api/test/functional/provider-earnings.spec.tsapps/api/src/billing/lib/wallet/wallet.tsapps/api/src/core/providers/postgres.provider.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/provider-deployments.spec.tsapps/api/src/core/services/feature-flags/feature-flags.service.spec.tsapps/api/test/functional/nodes-v1.spec.tsapps/api/test/functional/providers.spec.tsapps/api/test/functional/addresses.spec.tsapps/api/test/functional/dashboard-data.spec.tsapps/api/test/functional/provider-earnings.spec.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: baktun14
Repo: akash-network/console PR: 1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
📚 Learning: 2025-11-25T17:45:49.180Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-11-25T17:45:49.180Z
Learning: Applies to {apps/deploy-web,apps/provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations
Applied to files:
apps/api/test/functional/provider-deployments.spec.ts
📚 Learning: 2025-11-25T17:45:52.965Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-11-25T17:45:52.965Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files. The `setup` function must be at the bottom of the root `describe` block, 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.
Applied to files:
apps/api/test/functional/provider-deployments.spec.tsapps/api/src/core/services/feature-flags/feature-flags.service.spec.tsapps/api/test/functional/nodes-v1.spec.tsapps/api/test/functional/providers.spec.tsapps/api/test/functional/addresses.spec.tsapps/api/test/functional/dashboard-data.spec.tsapps/api/test/functional/provider-earnings.spec.ts
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: The tsyringe container in the Akash Network console project has been extended with a custom dispose() method that iterates over all registered instances and calls dispose() on each one that implements it, enabling proper resource cleanup during shutdown.
Applied to files:
apps/api/test/functional/provider-deployments.spec.tsapps/api/test/functional/nodes-v1.spec.tsapps/api/test/functional/providers.spec.tsapps/api/src/console.tsapps/api/test/functional/dashboard-data.spec.ts
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: In the Akash Network console project, services like JobQueueService implement dispose() methods for resource cleanup, and the tsyringe container has been extended with a dispose() method that iterates over all registered instances and calls their dispose() methods, enabling proper shutdown orchestration.
Applied to files:
apps/api/test/functional/provider-deployments.spec.tsapps/api/test/functional/nodes-v1.spec.tsapps/api/test/functional/providers.spec.tsapps/api/src/console.tsapps/api/src/core/providers/http-sdk.provider.tsapps/api/test/functional/dashboard-data.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/core/services/feature-flags/feature-flags.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/core/services/feature-flags/feature-flags.service.spec.tsapps/api/test/functional/addresses.spec.ts
📚 Learning: 2025-11-19T16:13:43.249Z
Learnt from: stalniy
Repo: akash-network/console PR: 2255
File: apps/api/src/middlewares/privateMiddleware.ts:5-9
Timestamp: 2025-11-19T16:13:43.249Z
Learning: In the Akash Console API (apps/api), avoid resolving DI container dependencies at module scope (module initialization time) to prevent side effects. Instead, resolve dependencies inside functions/methods where they are actually used, even if this means resolving on every invocation, to maintain explicit control over when side effects occur and improve testability.
Applied to files:
apps/api/test/functional/addresses.spec.ts
📚 Learning: 2025-05-28T20:42:58.200Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 1372
File: apps/api/src/dashboard/services/stats/stats.service.ts:0-0
Timestamp: 2025-05-28T20:42:58.200Z
Learning: The user successfully implemented CosmosHttpService with retry logic using axiosRetry, exponential backoff, and proper error handling for Cosmos API calls, replacing direct axios usage in StatsService.
Applied to files:
apps/api/src/billing/services/financial-stats/financial-stats.service.ts
📚 Learning: 2025-08-12T13:52:38.708Z
Learnt from: stalniy
Repo: akash-network/console PR: 1800
File: apps/deploy-web/next.config.js:163-165
Timestamp: 2025-08-12T13:52:38.708Z
Learning: In the Akash Console project, akashnetwork/env-loader is used at the top of next.config.js files to automatically load environment variables from env/.env files into process.env. SENTRY_ORG and SENTRY_PROJECT are stored as public configuration values in apps/deploy-web/env/.env and are loaded this way, while only SENTRY_AUTH_TOKEN is handled as a GitHub secret in workflows.
Applied to files:
apps/api/env/.env.mainnetapps/api/src/core/config/env.config.ts
📚 Learning: 2025-08-12T13:52:38.708Z
Learnt from: stalniy
Repo: akash-network/console PR: 1800
File: apps/deploy-web/next.config.js:163-165
Timestamp: 2025-08-12T13:52:38.708Z
Learning: In the Akash Console project, SENTRY_ORG and SENTRY_PROJECT are stored as public configuration values in apps/deploy-web/env/.env file and loaded by akashnetwork/env-loader, not as GitHub secrets. Only SENTRY_AUTH_TOKEN should be handled as a secret.
Applied to files:
apps/api/env/.env.mainnet
📚 Learning: 2025-09-25T20:34:55.117Z
Learnt from: jigar-arc10
Repo: akash-network/console PR: 1970
File: apps/provider-console/src/config/network.config.ts:17-28
Timestamp: 2025-09-25T20:34:55.117Z
Learning: In cosmos-kit integration, nodesUrl and versionUrl should point to Cosmos API endpoints (not console backend API endpoints) as cosmos-kit expects these to be Cosmos REST endpoints for proper functionality.
Applied to files:
apps/api/env/.env.mainnet
📚 Learning: 2025-06-10T01:25:37.604Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 1458
File: apps/api/src/network/services/network/network.service.ts:0-0
Timestamp: 2025-06-10T01:25:37.604Z
Learning: In the Akash Network Console codebase, network parameters are validated at the schema level using Zod schemas before reaching service methods, making default cases in switch statements unnecessary for validated enum-like parameters.
Applied to files:
apps/api/src/core/config/env.config.ts
📚 Learning: 2025-06-10T01:25:36.097Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 1458
File: apps/api/src/network/services/network/network.service.ts:0-0
Timestamp: 2025-06-10T01:25:36.097Z
Learning: In the Akash Console API, network parameter validation is handled at the HTTP schema level using Zod validation, making additional runtime validation checks in service layers unnecessary. The validation-at-the-boundary approach is preferred over defensive programming throughout every layer.
Applied to files:
apps/api/src/core/config/env.config.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/lib/wallet/wallet.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/lib/wallet/wallet.ts
🧬 Code graph analysis (6)
apps/api/src/core/services/feature-flags/feature-flags.service.spec.ts (2)
apps/api/src/core/config/env.config.ts (1)
CoreConfig(42-42)apps/api/src/core/providers/config.provider.ts (1)
CoreConfig(13-13)
apps/api/src/core/providers/config.provider.ts (1)
apps/api/src/core/config/env.config.ts (2)
CoreConfig(42-42)envSchema(4-40)
packages/http-sdk/src/block/block-http.service.ts (1)
packages/http-sdk/src/index.ts (1)
HttpClient(26-26)
apps/api/test/functional/addresses.spec.ts (1)
apps/api/src/core/providers/config.provider.ts (1)
CORE_CONFIG(7-7)
apps/api/src/core/config/env.config.ts (2)
packages/net/src/index.ts (1)
netConfig(3-3)apps/api/src/core/providers/config.provider.ts (1)
CoreConfig(13-13)
apps/api/src/core/providers/postgres.provider.ts (2)
apps/api/src/core/providers/config.provider.ts (2)
CORE_CONFIG(7-7)CoreConfig(13-13)apps/api/src/core/config/env.config.ts (1)
CoreConfig(42-42)
🪛 dotenv-linter (4.0.0)
apps/api/env/.env.mainnet
[warning] 2-2: [UnorderedKey] The REST_API_NODE_URL key should go before the RPC_NODE_ENDPOINT key
(UnorderedKey)
apps/api/env/.env.sandbox
[warning] 2-2: [UnorderedKey] The REST_API_NODE_URL key should go before the RPC_NODE_ENDPOINT key
(UnorderedKey)
[warning] 3-3: [UnorderedKey] The DEPLOYMENT_GRANT_DENOM key should go before the REST_API_NODE_URL key
(UnorderedKey)
⏰ 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). (8)
- GitHub Check: stats-web-ci
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
🔇 Additional comments (21)
apps/api/env/.env.sandbox (1)
2-2: Addition of REST_API_NODE_URL aligns with DI refactoring goals.The introduction of
REST_API_NODE_URLas an environment configuration ties into the CoreConfigService and enables the dependency-injection pattern outlined in the PR objectives. This supports proper encapsulation of HTTP client configuration.packages/http-sdk/src/block/block-http.service.ts (2)
1-2: Clean import refactoring for composition pattern.The use of
typeimport forHttpClientis appropriate and the imports are properly scoped for the new DI-based approach.
12-13: Good refactor to composition over inheritance.The constructor-based dependency injection with
private readonlyis a clean pattern that improves testability and follows SOLID principles.apps/api/src/billing/lib/wallet/wallet.ts (3)
7-35: Centralizing bech32 prefix and derivation options looks goodUsing a single
WALLET_ADDRESS_PREFIXconstant and routing allDirectSecp256k1HdWalletoptions throughgetInstanceOptionsis a nice encapsulation improvement and removes hard‑coded"akash"usage from call sites. Theindex === undefinedvs defined branches forhdPathsare clear and preserve existing behavior.
14-24: Private fields and constructor initialization are consistent and encapsulatedPromoting
instanceAsPromised,aminoSignerPromise, andwalletIndexto#private fields (with#instanceAsPromisedmarkedreadonly) cleanly hides internal state while keeping construction logic straightforward. Both mnemonic and non‑mnemonic paths fully initialize#instanceAsPromised, so there’s no uninitialized state at runtime.
38-45: Direct signer surface is thin and correctly delegated
getAccountsandsignDirectsimply delegate to the underlyingDirectSecp256k1HdWalletvia the cached#instanceAsPromisedpromise, which matches theOfflineDirectSignercontract and keeps this class as a light wrapper with good encapsulation.apps/api/test/functional/provider-earnings.spec.ts (1)
5-5: LGTM! DI-managed cleanup pattern is consistent with the broader refactoring.Using
container.dispose()properly delegates cleanup to the DI container, which handles closing database connections and other disposable resources.Also applies to: 37-39
apps/api/src/core/services/feature-flags/feature-flags.service.spec.ts (1)
5-5: LGTM! Type alignment with CoreConfig is correct.The import and parameter type changes properly reflect the migration to the new
CoreConfigtype from the DI-based configuration pattern.Also applies to: 166-167
apps/api/src/db/dbConnection.ts (1)
63-66: LGTM! Clean inline dispose implementation.The inline
dispose()method properly closes both databases concurrently and integrates well with the DI container's disposal lifecycle.apps/api/src/core/providers/postgres.provider.ts (3)
20-28: LGTM! Clean DI registration for the Postgres client.Using a Symbol as an internal injection token provides good encapsulation. The factory properly resolves config and creates the client with appropriate settings.
36-46: LGTM! Migration client lifecycle is properly managed.Creating a separate client for migrations with
max: 1connection and ensuring cleanup in thefinallyblock is a solid pattern that prevents connection leaks.
48-50: LGTM! Composition-based database creation.The
getPgDatabasefunction cleanly composes the drizzle instance from DI-resolved dependencies, and the type is correctly derived usingReturnType.Also applies to: 68-70, 79-79
apps/api/test/functional/providers.spec.ts (1)
7-7: LGTM! DI-managed cleanup is consistent with other functional tests.The switch to
container.dispose()aligns with the broader pattern established across the test suite.Also applies to: 79-81
apps/api/test/functional/dashboard-data.spec.ts (1)
3-3: Switch tocontainer.dispose()in functional test teardown looks correctUsing
container.dispose()for cleanup aligns with the DI-based lifecycle and centralizes resource disposal; no additional teardown is needed here.Also applies to: 197-199
apps/api/test/functional/provider-deployments.spec.ts (1)
5-5: DI-based teardown for provider deployments tests is consistentImporting
containerand usingawait container.dispose()inafterAllfits the new global shutdown pattern; keepingmcache.clear()afterward is fine.Also applies to: 113-115
apps/api/test/functional/nodes-v1.spec.ts (1)
4-4: Nodes API functional tests now follow the shared DI teardown patternUsing
container.dispose()plus cache clearing inafterAllis consistent with other functional tests and with the container’s custom dispose semantics.Also applies to: 18-21
apps/api/src/billing/services/financial-stats/financial-stats.service.ts (1)
2-2: Refactor to useCosmosHttpServicefor community pool is consistent with existing patternsInjecting
CosmosHttpServiceand implementinggetCommunityPoolUsdcviacosmosHttpService.getCommunityPool()plusparseFloatmatches how balances are already handled in this service and keeps HTTP access centralized in the shared client.Ensure
CosmosHttpService.getCommunityPool()indeed returns an array of coins{ denom, amount }as assumed here; if its return type changes, this method will need to be adjusted accordingly.Also applies to: 13-17, 49-51
apps/api/src/core/config/env.config.ts (1)
1-1:REST_API_NODE_URLaddition andCoreConfigtyping are consistent with the new config model
REST_API_NODE_URLis properly validated as a URL and given a network‑aware default vianetConfig.getBaseAPIUrl(process.env.NETWORK || "mainnet"), matching how the rest of the system now expects to resolve the Cosmos REST API base.- Exporting
type CoreConfig = z.infer<typeof envSchema>cleanly ties the DICORE_CONFIGtoken to this schema.This looks aligned with the broader CoreConfig/provider refactor.
Also applies to: 24-32, 42-42
apps/api/src/core/providers/http-sdk.provider.ts (2)
19-29: DI-based CHAIN_API_HTTP_CLIENT setup looks correct; confirm CoreConfigService registration orderUsing
CoreConfigServicewithinstancePerContainerCachingFactoryto resolveREST_API_NODE_URLgives a single, lazily createdHttpClientper container and keeps configuration centralized. Please just verify thatCoreConfigServiceis registered in the container before this provider file is imported/initialized, and that the config key string"REST_API_NODE_URL"matches theenvSchemafield name you expect.
39-45: NON_AXIOS_SERVICES typing and BlockHttpService registration align with shared HttpClient usageTyping
NON_AXIOS_SERVICESasArray<new (httpClient: HttpClient) => unknown>and constructing each withCHAIN_API_HTTP_CLIENTimproves type safety and makes the intent clear. AddingBlockHttpServicehere is consistent with services that accept anHttpClientdirectly and keeps them all sharing the same underlying chain client.apps/api/src/core/providers/config.provider.ts (1)
1-13: CORE_CONFIG provider correctly centralizes env parsing; ensure container lifecycle matches test/env needsDefining
CORE_CONFIGas anInjectionToken<CoreConfig>and registering it viainstancePerContainerCachingFactory(() => envSchema.parse(process.env))cleanly moves config to DI and guarantees a single validatedCoreConfigper container instance. One thing to double‑check: if any tests or runtime code mutateprocess.envbetween scenarios, they will now need to either recreate/replace the container or override theCORE_CONFIGregistration, since the parsed value is cached. Given the existingcontainer.dispose()pattern in this repo, that should be straightforward but is worth confirming in the test setup.
stalniy
force-pushed
the
refactor/master-wallet
branch
from
98e68d1 to
856919a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/api/env/.env.sandbox (1)
1-3: Consider alphabetical ordering of environment variables.Static analysis flagged the key ordering. While functional, alphabetically ordering env vars improves maintainability:
-RPC_NODE_ENDPOINT=https://rpc.sandbox-2.aksh.pw:443 -REST_API_NODE_URL=https://api.sandbox-2.aksh.pw:443 DEPLOYMENT_GRANT_DENOM=uakt +REST_API_NODE_URL=https://api.sandbox-2.aksh.pw:443 +RPC_NODE_ENDPOINT=https://rpc.sandbox-2.aksh.pw:443Based on learnings, this could be addressed in a separate issue if you prefer to keep the PR focused.
apps/api/test/functional/proposals.spec.ts (1)
98-277: Consider caching the resolved REST API URL in the setup function.The
container.resolve(CORE_CONFIG).REST_API_NODE_URLexpression is resolved 4 times within the samesetupfunction (lines 99, 226, 264, 276). Consider extracting it to a local constant at the beginning of the function to improve readability and avoid redundant resolution.Apply this refactor:
const setup = () => { + const restApiUrl = container.resolve(CORE_CONFIG).REST_API_NODE_URL; - nock(container.resolve(CORE_CONFIG).REST_API_NODE_URL) + nock(restApiUrl) .persist() .get("/cosmos/gov/v1beta1/proposals?pagination.limit=1000") // ... rest of mock - nock(container.resolve(CORE_CONFIG).REST_API_NODE_URL) + nock(restApiUrl) .persist() .get("/cosmos/gov/v1beta1/proposals/2") // ... rest of mock - nock(container.resolve(CORE_CONFIG).REST_API_NODE_URL) + nock(restApiUrl) .persist() .get("/cosmos/gov/v1beta1/proposals/2/tally") // ... rest of mock - nock(container.resolve(CORE_CONFIG).REST_API_NODE_URL).persist().get("/cosmos/gov/v1beta1/proposals/999").reply(404); + nock(restApiUrl).persist().get("/cosmos/gov/v1beta1/proposals/999").reply(404); };apps/api/env/.env.mainnet (1)
1-3: Optional: Consider reordering environment variables.The static analysis tool suggests alphabetical ordering of keys. While not critical, reordering
REST_API_NODE_URLbeforeRPC_NODE_ENDPOINTwould satisfy the linter and improve consistency.Apply this diff if desired:
+REST_API_NODE_URL=https://consoleapi.akashnet.net RPC_NODE_ENDPOINT=https://consolerpc.akashnet.net -REST_API_NODE_URL=https://consoleapi.akashnet.net DEPLOYMENT_GRANT_DENOM=ibc/170C677610AC31DF0904FFE09CD3B5C657492170E7E52372E48756B71E56F2F1apps/api/test/functional/addresses.spec.ts (1)
227-227: ExtractREST_API_NODE_URLto a local variable to avoid repeated DI resolution.The
container.resolve(CORE_CONFIG).REST_API_NODE_URLis called 7 times in the setup function. Extract it once at the start for clarity and to avoid redundant resolutions.const setup = async () => { await connectUsingSequelize(); + const restApiNodeUrl = container.resolve(CORE_CONFIG).REST_API_NODE_URL; const address = createAkashAddress(); const validators = await Promise.all([ createValidator({ accountAddress: address }), createValidator() ]); - nock(container.resolve(CORE_CONFIG).REST_API_NODE_URL) + nock(restApiNodeUrl) .persist() .get(`/cosmos/bank/v1beta1/balances/${address}?pagination.limit=1000`)Apply similar changes to all other
nock(container.resolve(CORE_CONFIG).REST_API_NODE_URL)calls.apps/api/src/chain/services/block-http/block-http.service.spec.ts (1)
19-23: Consider adding a parameter for guideline consistency.The setup function follows most guidelines but is missing a parameter. As per coding guidelines, the setup function should "accept a single parameter with inline type definition" for consistency and extensibility, even if not currently needed.
Apply this diff to align with the guideline:
- function setup() { + function setup(options: { height?: number } = {}) { const blockHttpService = mock<BlockHttpServiceCommon>(); const service = new BlockHttpService(blockHttpService); return { blockHttpService, service }; }Based on coding guidelines, which specify that setup functions should accept a single parameter with inline type definition.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (42)
apps/api/env/.env.mainnet(1 hunks)apps/api/env/.env.sandbox(1 hunks)apps/api/src/billing/lib/wallet/wallet.ts(1 hunks)apps/api/src/billing/services/financial-stats/financial-stats.service.ts(2 hunks)apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts(0 hunks)apps/api/src/chain/services/block-http/block-http.service.spec.ts(1 hunks)apps/api/src/console.ts(1 hunks)apps/api/src/core/config/env.config.ts(3 hunks)apps/api/src/core/config/index.ts(0 hunks)apps/api/src/core/providers/config.provider.ts(1 hunks)apps/api/src/core/providers/http-sdk.provider.ts(1 hunks)apps/api/src/core/providers/postgres.provider.ts(2 hunks)apps/api/src/core/services/feature-flags/feature-flags.service.spec.ts(2 hunks)apps/api/src/db/dbConnection.ts(1 hunks)apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts(3 hunks)apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.ts(0 hunks)apps/api/src/utils/constants.ts(0 hunks)apps/api/test/custom-jest-environment.ts(1 hunks)apps/api/test/functional/addresses.spec.ts(9 hunks)apps/api/test/functional/balances.spec.ts(4 hunks)apps/api/test/functional/bids.spec.ts(2 hunks)apps/api/test/functional/create-deployment.spec.ts(3 hunks)apps/api/test/functional/dashboard-data.spec.ts(2 hunks)apps/api/test/functional/deployments.spec.ts(8 hunks)apps/api/test/functional/gpu.spec.ts(2 hunks)apps/api/test/functional/graph-data.spec.ts(2 hunks)apps/api/test/functional/managed-api-deployment-flow.spec.ts(2 hunks)apps/api/test/functional/network-capacity.spec.ts(2 hunks)apps/api/test/functional/nodes-v1.spec.ts(2 hunks)apps/api/test/functional/proposals.spec.ts(5 hunks)apps/api/test/functional/provider-deployments.spec.ts(2 hunks)apps/api/test/functional/provider-earnings.spec.ts(2 hunks)apps/api/test/functional/provider-graph-data.spec.ts(2 hunks)apps/api/test/functional/providers.spec.ts(2 hunks)apps/api/test/functional/validators.spec.ts(4 hunks)apps/api/test/services/test-database.service.ts(1 hunks)apps/api/test/services/test-wallet.service.ts(4 hunks)apps/api/test/setup-functional-tests.ts(2 hunks)apps/api/test/setup-unit-tests.ts(1 hunks)apps/provider-proxy/package.json(1 hunks)package.json(1 hunks)packages/http-sdk/src/block/block-http.service.ts(2 hunks)
💤 Files with no reviewable changes (4)
- apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.ts
- apps/api/src/utils/constants.ts
- apps/api/src/core/config/index.ts
- apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts
🚧 Files skipped from review as they are similar to previous changes (19)
- apps/api/test/functional/validators.spec.ts
- apps/api/test/functional/provider-graph-data.spec.ts
- apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts
- apps/provider-proxy/package.json
- apps/api/test/functional/balances.spec.ts
- apps/api/test/functional/provider-deployments.spec.ts
- apps/api/test/functional/dashboard-data.spec.ts
- apps/api/test/functional/gpu.spec.ts
- apps/api/test/functional/create-deployment.spec.ts
- apps/api/test/functional/graph-data.spec.ts
- apps/api/test/functional/providers.spec.ts
- apps/api/src/core/config/env.config.ts
- apps/api/test/functional/deployments.spec.ts
- apps/api/test/functional/nodes-v1.spec.ts
- apps/api/test/setup-unit-tests.ts
- package.json
- apps/api/test/functional/managed-api-deployment-flow.spec.ts
- apps/api/test/functional/network-capacity.spec.ts
- apps/api/src/core/services/feature-flags/feature-flags.service.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/test/services/test-database.service.tsapps/api/test/functional/provider-earnings.spec.tsapps/api/src/db/dbConnection.tspackages/http-sdk/src/block/block-http.service.tsapps/api/test/functional/proposals.spec.tsapps/api/test/setup-functional-tests.tsapps/api/src/chain/services/block-http/block-http.service.spec.tsapps/api/src/billing/services/financial-stats/financial-stats.service.tsapps/api/test/custom-jest-environment.tsapps/api/src/core/providers/postgres.provider.tsapps/api/src/core/providers/config.provider.tsapps/api/test/functional/addresses.spec.tsapps/api/test/services/test-wallet.service.tsapps/api/src/console.tsapps/api/src/billing/lib/wallet/wallet.tsapps/api/src/core/providers/http-sdk.provider.tsapps/api/test/functional/bids.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/provider-earnings.spec.tsapps/api/test/functional/proposals.spec.tsapps/api/src/chain/services/block-http/block-http.service.spec.tsapps/api/test/functional/addresses.spec.tsapps/api/test/functional/bids.spec.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: baktun14
Repo: akash-network/console PR: 1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
📚 Learning: 2025-11-25T17:45:52.965Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-11-25T17:45:52.965Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files. The `setup` function must be at the bottom of the root `describe` block, 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.
Applied to files:
apps/api/test/functional/provider-earnings.spec.tsapps/api/test/setup-functional-tests.tsapps/api/src/chain/services/block-http/block-http.service.spec.tsapps/api/test/functional/addresses.spec.tsapps/api/test/functional/bids.spec.ts
📚 Learning: 2025-06-08T03:07:13.871Z
Learnt from: stalniy
Repo: akash-network/console PR: 1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Applied to files:
apps/api/test/functional/provider-earnings.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/test/functional/proposals.spec.tsapps/api/src/chain/services/block-http/block-http.service.spec.tsapps/api/test/functional/addresses.spec.tsapps/api/test/functional/bids.spec.ts
📚 Learning: 2025-11-25T17:45:58.258Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/test-descriptions.mdc:0-0
Timestamp: 2025-11-25T17:45:58.258Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `<Subject>.name` in the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Applied to files:
apps/api/test/functional/proposals.spec.ts
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: The tsyringe container in the Akash Network console project has been extended with a custom dispose() method that iterates over all registered instances and calls dispose() on each one that implements it, enabling proper resource cleanup during shutdown.
Applied to files:
apps/api/test/setup-functional-tests.tsapps/api/src/console.ts
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: In the Akash Network console project, services like JobQueueService implement dispose() methods for resource cleanup, and the tsyringe container has been extended with a dispose() method that iterates over all registered instances and calls their dispose() methods, enabling proper shutdown orchestration.
Applied to files:
apps/api/test/setup-functional-tests.tsapps/api/src/console.tsapps/api/src/core/providers/http-sdk.provider.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/chain/services/block-http/block-http.service.spec.tsapps/api/test/custom-jest-environment.ts
📚 Learning: 2025-05-28T20:42:58.200Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 1372
File: apps/api/src/dashboard/services/stats/stats.service.ts:0-0
Timestamp: 2025-05-28T20:42:58.200Z
Learning: The user successfully implemented CosmosHttpService with retry logic using axiosRetry, exponential backoff, and proper error handling for Cosmos API calls, replacing direct axios usage in StatsService.
Applied to files:
apps/api/src/billing/services/financial-stats/financial-stats.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/test/custom-jest-environment.tsapps/api/test/services/test-wallet.service.tsapps/api/src/billing/lib/wallet/wallet.ts
📚 Learning: 2025-11-19T16:13:43.249Z
Learnt from: stalniy
Repo: akash-network/console PR: 2255
File: apps/api/src/middlewares/privateMiddleware.ts:5-9
Timestamp: 2025-11-19T16:13:43.249Z
Learning: In the Akash Console API (apps/api), avoid resolving DI container dependencies at module scope (module initialization time) to prevent side effects. Instead, resolve dependencies inside functions/methods where they are actually used, even if this means resolving on every invocation, to maintain explicit control over when side effects occur and improve testability.
Applied to files:
apps/api/test/functional/addresses.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/lib/wallet/wallet.ts
📚 Learning: 2025-08-12T13:52:38.708Z
Learnt from: stalniy
Repo: akash-network/console PR: 1800
File: apps/deploy-web/next.config.js:163-165
Timestamp: 2025-08-12T13:52:38.708Z
Learning: In the Akash Console project, akashnetwork/env-loader is used at the top of next.config.js files to automatically load environment variables from env/.env files into process.env. SENTRY_ORG and SENTRY_PROJECT are stored as public configuration values in apps/deploy-web/env/.env and are loaded this way, while only SENTRY_AUTH_TOKEN is handled as a GitHub secret in workflows.
Applied to files:
apps/api/env/.env.mainnet
📚 Learning: 2025-06-10T14:19:14.122Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 1461
File: apps/notifications/src/modules/alert/services/deployment-alert/deployment-alert.service.ts:194-197
Timestamp: 2025-06-10T14:19:14.122Z
Learning: The alert.CONSOLE_WEB_URL environment variable in the notifications app is controlled and configured by the Akash team, reducing XSS concerns for HTML link generation in alert messages.
Applied to files:
apps/api/env/.env.mainnet
📚 Learning: 2025-09-25T20:34:55.117Z
Learnt from: jigar-arc10
Repo: akash-network/console PR: 1970
File: apps/provider-console/src/config/network.config.ts:17-28
Timestamp: 2025-09-25T20:34:55.117Z
Learning: In cosmos-kit integration, nodesUrl and versionUrl should point to Cosmos API endpoints (not console backend API endpoints) as cosmos-kit expects these to be Cosmos REST endpoints for proper functionality.
Applied to files:
apps/api/env/.env.mainnet
🧬 Code graph analysis (8)
packages/http-sdk/src/block/block-http.service.ts (1)
packages/http-sdk/src/index.ts (1)
HttpClient(26-26)
apps/api/test/functional/proposals.spec.ts (1)
apps/api/src/core/providers/config.provider.ts (1)
CORE_CONFIG(7-7)
apps/api/src/chain/services/block-http/block-http.service.spec.ts (1)
packages/http-sdk/src/block/block-http.service.ts (1)
BlockHttpService(12-20)
apps/api/src/core/providers/config.provider.ts (2)
apps/api/src/core/config/env.config.ts (2)
CoreConfig(42-42)envSchema(4-40)apps/api/src/billing/config/env.config.ts (1)
envSchema(8-37)
apps/api/test/functional/addresses.spec.ts (1)
apps/api/src/core/providers/config.provider.ts (1)
CORE_CONFIG(7-7)
apps/api/test/services/test-wallet.service.ts (1)
apps/api/src/billing/lib/wallet/wallet.ts (2)
WALLET_ADDRESS_PREFIX(7-7)Wallet(9-57)
apps/api/src/core/providers/http-sdk.provider.ts (4)
packages/http-sdk/src/index.ts (2)
HttpClient(26-26)createHttpClient(26-26)packages/http-sdk/src/utils/httpClient.ts (2)
HttpClient(23-23)createHttpClient(5-21)packages/http-sdk/src/balance/balance-http.service.ts (1)
BalanceHttpService(20-29)packages/http-sdk/src/block/block-http.service.ts (1)
BlockHttpService(12-20)
apps/api/test/functional/bids.spec.ts (1)
apps/api/src/core/providers/config.provider.ts (1)
CORE_CONFIG(7-7)
🪛 dotenv-linter (4.0.0)
apps/api/env/.env.sandbox
[warning] 2-2: [UnorderedKey] The REST_API_NODE_URL key should go before the RPC_NODE_ENDPOINT key
(UnorderedKey)
[warning] 3-3: [UnorderedKey] The DEPLOYMENT_GRANT_DENOM key should go before the REST_API_NODE_URL key
(UnorderedKey)
apps/api/env/.env.mainnet
[warning] 2-2: [UnorderedKey] The REST_API_NODE_URL key should go before the RPC_NODE_ENDPOINT key
(UnorderedKey)
⏰ 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). (13)
- GitHub Check: validate (apps/log-collector) / validate-unsafe
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (25)
apps/api/test/setup-functional-tests.ts (1)
40-48: Graceful disposal handling looks good.The try/catch around
container.dispose()correctly handles cases where the container may have already been disposed by individual tests, preventing spurious errors during teardown. Based on learnings, the tsyringe container has been extended with a customdispose()method that handles resource cleanup.apps/api/src/console.ts (1)
107-109: Shutdown simplification aligns with DI-managed resource lifecycle.Delegating all cleanup to
container.dispose()is appropriate given that the tsyringe container has been extended to calldispose()on all registered instances. This centralizes resource cleanup and maintains consistency with the DI-driven teardown pattern used elsewhere in the codebase. Based on learnings about the container's custom dispose behavior.apps/api/src/billing/lib/wallet/wallet.ts (3)
7-8: Good extraction of the address prefix constant.Exporting
WALLET_ADDRESS_PREFIXenables consistent usage across the codebase (e.g., test utilities) while keeping the value centralized in the wallet module.
14-16: Private class fields improve encapsulation.Using ES private class fields (
#) instead of TypeScript'sprivatekeyword provides true runtime encapsulation — these fields are inaccessible outside the class, even through reflection or type assertions.
46-51: Lazy initialization pattern is correctly implemented.The
??=operator assigns the Promise object synchronously before anyawait, making this pattern safe for concurrent calls tosignAmino. All callers receive the same cached Promise instance.apps/api/test/services/test-wallet.service.ts (2)
57-60: Mnemonic generation properly decoupled from Wallet class.This addresses the previous review feedback to generate mnemonics separately, enabling removal of
getMnemonic()from theWalletclass and improving its encapsulation.
70-79: Wallet initialization flow correctly updated.The flow now generates the mnemonic first and explicitly passes it to the
Walletconstructor. This makes the data flow clearer and maintains the mnemonic locally for caching purposes.apps/api/test/custom-jest-environment.ts (1)
8-19: LGTM! Good encapsulation with private fields and lazy initialization.The refactoring correctly uses private class fields with the
#prefix and implements a lazy initialization pattern forTestWalletService. This improves encapsulation and defers instantiation until needed.apps/api/test/functional/provider-earnings.spec.ts (1)
37-38: LGTM! Proper DI container cleanup.The migration from
closeConnections()tocontainer.dispose()aligns with the broader pattern of DI-managed lifecycle across the test suite.apps/api/test/functional/bids.spec.ts (1)
3-5: LGTM! Proper DI-driven configuration.The integration of
CORE_CONFIGfor REST API URL resolution aligns with the broader refactoring toward centralized configuration management.Also applies to: 50-50
apps/api/src/db/dbConnection.ts (1)
63-65: LGTM! Improved encapsulation of disposal logic.Inlining the disposal logic within the
APP_INITIALIZERimproves encapsulation by removing the publicly exportedcloseConnectionsfunction and managing the lifecycle directly through the DI container.apps/api/test/services/test-database.service.ts (1)
47-51: LGTM! Proper resource cleanup with try-finally.Wrapping the migration in a try-finally block ensures the
migrationClientis properly closed even if the migration fails, preventing potential connection leaks.apps/api/src/billing/services/financial-stats/financial-stats.service.ts (1)
49-52: LGTM! Better encapsulation using CosmosHttpService.Replacing the manual HTTP call with
cosmosHttpService.getCommunityPool()improves encapsulation and aligns with the broader refactoring toward service-based HTTP access. The explicit return type annotation is also good practice.apps/api/test/functional/addresses.spec.ts (1)
41-44: LGTM - proper cleanup using container disposal.The switch to
container.dispose()correctly aligns with the DI-managed lifecycle pattern introduced in this PR.apps/api/src/core/providers/config.provider.ts (1)
1-13: LGTM - Clean factory-based config registration with proper caching.The transition from static config value to a factory with
instancePerContainerCachingFactoryenables lazy initialization and proper container lifecycle management while maintaining singleton semantics.apps/api/src/core/providers/postgres.provider.ts (3)
20-28: LGTM - Well-structured DI registration with disposable lifecycle.The
APP_PG_CLIENTregistration correctly usesDisposableRegistry.registerFromFactoryto ensure the postgres client is properly disposed when the container is disposed. The Symbol-based token keeps this as an internal implementation detail.
36-46: LGTM - Proper migration client lifecycle management.The
migratePGfunction correctly creates a dedicated short-lived client for migrations withmax: 1connection, executes migrations, and ensures cleanup viafinallyblock regardless of success or failure.
48-50: LGTM - Clean database factory pattern.The
getPgDatabasehelper and its usage inPOSTGRES_DBregistration follows proper DI patterns. TheApiPgDatabasetype correctly infers from the factory return type, ensuring type consistency.Also applies to: 68-70, 79-79
apps/api/src/core/providers/http-sdk.provider.ts (2)
24-30: LGTM - Proper DI-based HTTP client configuration.The
CHAIN_API_HTTP_CLIENTfactory correctly resolves the REST API URL fromCoreConfigServiceat instantiation time, maintaining proper encapsulation.
32-47: LGTM - Correct service categorization based on constructor signatures.The separation is appropriate:
SERVICEScontains services that accept a config object withbaseURL, whileNON_AXIOS_SERVICEScontains services that accept anHttpClientinstance. MovingBlockHttpServicetoNON_AXIOS_SERVICEScorrectly reflects its constructor signature(httpClient: HttpClient).packages/http-sdk/src/block/block-http.service.ts (3)
1-2: LGTM! Imports support the composition refactor.The new imports correctly provide the
extractDatahelper andHttpClienttype needed for the dependency injection approach.
12-13: LGTM! Excellent refactor to composition pattern.The change from inheritance to composition improves testability and follows the Dependency Inversion Principle. The injected
HttpClientdependency is correctly declared withprivate readonly.
15-16: LGTM! Method correctly adapted for composition.The implementation properly uses the injected
httpClientdependency and theextractDatahelper function while preserving the original logic and return type.apps/api/src/chain/services/block-http/block-http.service.spec.ts (2)
3-5: LGTM! Imports follow coding guidelines.The type-only import for
BlockHttpServiceCommonis appropriate, and usingjest-mock-extendedfor mocking aligns with the project's testing standards.
10-17: LGTM! Test correctly uses setup function and mocking.The test properly calls
setup()to obtain the service and its dependency mock, then verifies the expected behavior. The mock setup and assertions are correct.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/api/src/core/providers/postgres.provider.ts (1)
36-46: Consider accepting container as parameter for testability.The function uses the global
containerdirectly, which works for startup migrations but makes isolated testing with child containers harder. Since migrations typically run once at startup, this is acceptable, but you could optionally accept aDependencyContainerparameter for consistency with the DI pattern used elsewhere.-export async function migratePG(): Promise<void> { - const config = container.resolve(CORE_CONFIG); +export async function migratePG(c: DependencyContainer = container): Promise<void> { + const config = c.resolve(CORE_CONFIG);apps/api/src/core/lib/disposable-registry/disposable-registry.ts (1)
40-49: Optional: Remove redundant type cast.The
isDisposable()type guard already narrowsvaluetoDisposablewithin the if block, making theas Disposablecast unnecessary.if (this.isDisposable(value)) { - this.register(value as Disposable); + this.register(value); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/src/core/lib/disposable-registry/disposable-registry.ts(3 hunks)apps/api/src/core/providers/postgres.provider.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/core/providers/postgres.provider.tsapps/api/src/core/lib/disposable-registry/disposable-registry.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: baktun14
Repo: akash-network/console PR: 1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: The tsyringe container in the Akash Network console project has been extended with a custom dispose() method that iterates over all registered instances and calls dispose() on each one that implements it, enabling proper resource cleanup during shutdown.
Applied to files:
apps/api/src/core/lib/disposable-registry/disposable-registry.ts
⏰ 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). (13)
- GitHub Check: validate (apps/api) / validate-unsafe
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
apps/api/src/core/providers/postgres.provider.ts (7)
6-16: LGTM!Imports are correctly updated to support the new DI-driven architecture with proper type-only imports for
DependencyContainer,InjectionToken, andCoreConfig.
20-28: Well-structured DI registration with proper lifecycle management.The encapsulated
APP_PG_CLIENTtoken, caching factory, and disposal registration viaDisposableRegistryalign well with the PR's encapsulation goals. The client is lazily initialized and properly cleaned up.
30-34: LGTM!Clean extraction of drizzle configuration with appropriate type narrowing using
Pick<CoreConfig, "SQL_LOG_FORMAT">.
48-50: LGTM!Clean internal helper that properly resolves dependencies from the provided container.
56-65: LGTM!The health check factory properly resolves the DI-managed client and uses
satisfiesfor type-safe object construction.
67-70: LGTM!Consistent factory-based registration with caching.
79-79: LGTM!Using
ReturnType<typeof getPgDatabase>ensures the type stays synchronized with the factory implementation.apps/api/src/core/lib/disposable-registry/disposable-registry.ts (3)
27-28: Excellent encapsulation improvement.The migration to hard-private fields (
#) strengthens encapsulation and aligns perfectly with the PR's objective to improve encapsulation. These fields are now truly private and cannot be accessed from outside the class.
51-53: Clean and straightforward implementation.The
registermethod provides a clear, explicit API for tracking disposables. The implementation is simple and correct for the typical lifecycle (registration at startup, disposal at shutdown).
70-88: Robust disposal implementation.The disposal logic correctly guards against multiple calls, disposes all resources concurrently with proper error aggregation, and cleans up the internal state. The synchronous flag check and set ensure correct behavior in JavaScript's event loop.
stalniy
changed the title
Why
To cleanup and improve encapsulation
Summary by CodeRabbit
New Features
Improvements
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.