feat(network): rpc proxy for server apps #1823
Conversation
WalkthroughConsolidates API/RPC URL resolution around proxy-aware configuration. Adds PROXY_API_URL/PROXY_RPC_URL envs and a use_proxy_urls feature flag; introduces ChainNetworkConfigService and RpcUrlResolverService; updates providers, services, tests, net config, web config, and feature-flagging to prefer proxy URLs when enabled. Removes legacy REST_API_NODE_URL / RPC_NODE_ENDPOINT usages. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Service as API Service
participant FF as FeatureFlagsService
participant CNC as ChainNetworkConfigService
participant Net as NetConfig
participant SDK as Http SDK
Service->>CNC: getBaseAPIUrl()
CNC->>FF: isEnabled("use_proxy_urls")
FF-->>CNC: boolean
CNC->>Net: netConfig.getBaseAPIUrl(network)
Net-->>CNC: apiUrl (proxy or direct)
CNC-->>Service: base API URL
Service->>SDK: new HttpService({ baseURL })
sequenceDiagram
autonumber
actor Notifications as Notifications Module
participant FF as FeatureFlagsService
participant Cfg as ConfigService
participant R as RpcUrlResolverService
participant Client as Stargate/Tendermint Client
Notifications->>R: getRpcUrl()
R->>FF: isEnabled("use_proxy_urls")
alt enabled
R->>Cfg: getOrThrow("chain.PROXY_RPC_URL")
else
R->>Cfg: getOrThrow("chain.RPC_NODE_ENDPOINT")
end
R-->>Notifications: rpcUrl
Notifications->>Client: connect(rpcUrl)
sequenceDiagram
autonumber
actor GPU as GpuBidsCreator
participant CNC as ChainNetworkConfigService
participant Sig as SigningStargateClient
participant REST as REST API
GPU->>CNC: getBaseRpcUrl()
CNC-->>GPU: rpcUrl
alt rpcUrl missing
GPU-->>GPU: throw "RPC url for the GPU bids creator is not set."
else
GPU->>Sig: connectWithSigner(rpcUrl, signer)
Sig-->>GPU: client
GPU->>CNC: getBaseAPIUrl()
CNC-->>GPU: apiUrl
GPU->>REST: GET {apiUrl}/blocks/latest
REST-->>GPU: block height
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (2)
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts (1)
108-111: Replace ‘any’ with a concrete type (violates TS guidelines)Use SpendingAuthorizationMsgOptions here for correctness and maintainability.
Apply this diff:
- private async authorizeDeploymentSpending(options: any) { + private async authorizeDeploymentSpending(options: SpendingAuthorizationMsgOptions) { const deploymentAllowanceMsg = this.rpcMessageService.getDepositDeploymentGrantMsg(options); return await this.managedSignerService.executeRootTx([deploymentAllowanceMsg]); }apps/api/src/validator/services/validator/validator.service.ts (1)
43-50: Replaceanyin catch with a proper type guardPer guidelines, avoid
any. Use axios' isAxiosError to narrow the error type.Apply this diff to the catch block:
- } catch (error: any) { - if (error.response?.status === 404) { + } catch (error: unknown) { + if (isAxiosError(error) && error.response?.status === 404) { return null; } throw error; }And add this import at the top of the file:
import { isAxiosError } from "axios";
🧹 Nitpick comments (53)
packages/http-sdk/src/cosmos/cosmos-http.service.ts (2)
37-47: Replace console.log with structured/debug logging and compute URL robustly; include retry attempt countConsole logging in a shared SDK can be noisy and may leak full URLs into prod logs. Prefer debug-level logging and safer URL composition. Also, with retries enabled, each attempt will trigger this interceptor—include attempt info to aid debugging.
Apply this diff to improve logging and avoid “undefinedundefined” URL edge cases:
- // Add request logging interceptor - this.interceptors.request.use( - config => { - const fullUrl = `${config.baseURL}${config.url}`; - console.log(`[CosmosHttpService] Calling: ${fullUrl}`); - return config; - }, - error => { - return Promise.reject(error); - } - ); + // Add request logging interceptor + this.interceptors.request.use( + (config) => { + const method = (config.method ?? "GET").toUpperCase(); + const base = String(config.baseURL ?? ""); + const path = String(config.url ?? ""); + const fullUrl = + base && path ? `${base.replace(/\/$/, "")}/${path.replace(/^\//, "")}` : base || path; + const { retryCount = 0 } = axiosRetry.getCurrentState(config) ?? {}; + const attempt = retryCount + 1; + if (process.env.NODE_ENV !== "production") { + console.debug(`[CosmosHttpService] ${method} ${fullUrl} (attempt ${attempt}/${RETRY_COUNT + 1})`); + } + return config; + }, + (error) => Promise.reject(error), + );If a centralized logger is available for this package, prefer using it at debug level instead of console.debug.
30-36: Harden CosmosHttpService retry configHttpService in
packages/http-sdk/src/http/http.service.tsextends Axios (includingdefaults,interceptors, etc.), so castingthistoAxiosInstanceis safe—no extra indirection needed. To make retries on your Cosmos reads more robust, update the retry options to:
- Retry on network errors, 5xx, and 429 (rate limits)
- Only retry GET requests
- Reset the timeout between attempts
Apply this diff in
packages/http-sdk/src/cosmos/cosmos-http.service.ts:- axiosRetry(this as unknown as AxiosInstance, { - retries: RETRY_COUNT, - retryDelay: retryCount => Math.pow(2, retryCount) * RETRY_DELAY_MILLISECONDS, - retryCondition: (error: AxiosError) => axiosRetry.isNetworkError(error) || (error.response?.status !== undefined && error.response.status >= 500) - }); + axiosRetry(this, { + retries: RETRY_COUNT, + retryDelay: retryCount => Math.pow(2, retryCount) * RETRY_DELAY_MILLISECONDS, + // Network errors, 5xx, and 429 (rate limiting) + retryCondition: (error: AxiosError) => + axiosRetry.isNetworkOrIdempotentRequestError(error) || + error.response?.status === 429, + // Cosmos calls here are reads: only retry GET + methodsToRetry: ['GET'], + // Reset timeout between retries + shouldResetTimeout: true, + });apps/notifications/src/modules/chain/config/env.config.ts (1)
5-7: Optional: ParseUSE_PROXY_URLSas boolean and enforce URL formats
To catch mis-configured endpoints earlier, consider tightening the zod schema in
apps/notifications/src/modules/chain/config/env.config.ts:- RPC_NODE_ENDPOINT: z.string(), - USE_PROXY_URLS: z.enum(["true", "false"]).optional().default("false"), - PROXY_RPC_URL: z.string().optional().default("https://rpc.akt.dev/rpc") + RPC_NODE_ENDPOINT: z.string().url(), + USE_PROXY_URLS: z.coerce.boolean().optional().default(false), + PROXY_RPC_URL: z.string().url().optional().default("https://rpc.akt.dev/rpc")• We ran a search across
apps/notificationsand found no comparisons against"true"/"false", so switching to a boolean coercion should be safe.
• Be sure your Zod version supportsz.coerce.boolean().apps/api/src/utils/env.ts (1)
10-11: Tighten PROXY URL validation & confirm REST_API_NODE_URL removalNo references to
REST_API_NODE_URLremain inapps/api(verified via ripgrep). To strengthen your Zod schema, add.url()to both proxy entries:• File: apps/api/src/utils/env.ts
• Lines: 10–11apps/api/src/utils/env.ts @@ - PROXY_API_URL: z.string().optional().default("https://rpc.akt.dev/rest"), - PROXY_RPC_URL: z.string().optional().default("https://rpc.akt.dev/rpc"), + PROXY_API_URL: z.string().url().optional().default("https://rpc.akt.dev/rest"), + PROXY_RPC_URL: z.string().url().optional().default("https://rpc.akt.dev/rpc"),apps/api/test/functional/start-trial.spec.ts (1)
13-13: Prefer barrel import for wrappersImport
AuthzHttpServiceWrapperfrom the core services barrel for better maintainability and fewer deep-path references.Apply this diff:
-import { AuthzHttpServiceWrapper } from "@src/core/services/http-service-wrapper/http-service-wrapper"; +import { AuthzHttpServiceWrapper } from "@src/core/services";apps/stats-web/src/config/env-config.schema.ts (2)
15-16: Validate proxy URLs with zod.url()The new browser env fields are plain strings; using
.url()aligns with the rest of the schema and prevents silent acceptance of invalid URLs.Apply this diff:
- NEXT_PUBLIC_PROXY_API_URL: z.string().optional().default("https://rpc.akt.dev/rest"), - NEXT_PUBLIC_PROXY_RPC_URL: z.string().optional().default("https://rpc.akt.dev/rpc"), + NEXT_PUBLIC_PROXY_API_URL: z.string().url().optional().default("https://rpc.akt.dev/rest"), + NEXT_PUBLIC_PROXY_RPC_URL: z.string().url().optional().default("https://rpc.akt.dev/rpc"),
25-26: Server proxy URLs should also be validated as URLsSame rationale as the browser schema—use
.url()to catch misconfigurations early.Apply this diff:
- PROXY_API_URL: z.string().optional().default("https://rpc.akt.dev/rest"), - PROXY_RPC_URL: z.string().optional().default("https://rpc.akt.dev/rpc") + PROXY_API_URL: z.string().url().optional().default("https://rpc.akt.dev/rest"), + PROXY_RPC_URL: z.string().url().optional().default("https://rpc.akt.dev/rpc")apps/api/test/functional/create-deployment.spec.ts (1)
36-36: AvoidallowUnmocked: trueto prevent accidental live network callsAllowing unmocked requests can hide missing interceptors and introduce flakiness. Prefer explicit mocks and fail fast when an unexpected request occurs.
Apply this diff:
- nock(apiProxyUrl, { allowUnmocked: true }) + nock(apiProxyUrl)Additionally, consider updating
getCurrentHeight()to useapiProxyUrland mock it to keep tests hermetic. Example changes (outside the shown hunk):TypeScript (outside selected lines):
// Add near other imports import { apiProxyUrl } from "@src/utils/constants";And mock the height endpoint in the test before calling the API:
nock(apiProxyUrl).get("/blocks/latest").reply(200, { block: { header: { height: "12345" } } });Refactor getCurrentHeight (outside selected lines):
async function getCurrentHeight() { const response = await axios.get(`${apiProxyUrl}/blocks/latest`); return response.data.block.header.height; }apps/api/test/functional/bids.spec.ts (1)
83-83: Ensure no live network calls slip throughThe nock base is now
apiProxyUrl. Make sure all requests triggered by this test are covered by nocks; otherwise, they will fail. If additional endpoints are hit, add explicitnock(apiProxyUrl)interceptors rather than permitting unmocked calls.You can scan this spec for unmocked calls by enabling nock’s net connect enforcement at the start of the test run:
// e.g., in a global test setup file import nock from "nock"; nock.disableNetConnect();And selectively allow localhost if needed:
nock.enableNetConnect("127.0.0.1");apps/api/test/functional/balances.spec.ts (1)
163-173: Updated nock scopes to proxy base — good; consider hardening tests against query changesUsing nock(apiProxyUrl) is correct. To make tests resilient to query param order/additions across HTTP wrappers, prefer regex paths (as used elsewhere) over exact strings when query strings are involved.
Apply this pattern going forward:
- Use .get(new RegExp('/cosmos/.../endpoint.*')) when queries are present
- Keep .persist() and response bodies as-is
Also applies to: 175-186, 187-197
apps/api/test/functional/addresses.spec.ts (1)
225-239: Switching mocks to apiProxyUrl is correct; make GET matchers resilient to query param driftSeveral GET matchers use exact strings with query params, which can be brittle if the client adds/reorders parameters. Consider regex-based paths to reduce flakiness.
Apply diffs like the following to use RegExp-based matchers:
- nock(apiProxyUrl) - .persist() - .get(`/cosmos/bank/v1beta1/balances/${address}?pagination.limit=1000`) + nock(apiProxyUrl) + .persist() + .get(new RegExp(`/cosmos/bank/v1beta1/balances/${address}.*`)) .reply(200, {- nock(apiProxyUrl) - .persist() - .get(`/cosmos/staking/v1beta1/delegations/${address}?pagination.limit=1000`) + nock(apiProxyUrl) + .persist() + .get(new RegExp(`/cosmos/staking/v1beta1/delegations/${address}.*`)) .reply(200, {- nock(apiProxyUrl) - .persist() - .get(`/cosmos/staking/v1beta1/delegators/${address}/redelegations?pagination.limit=1000`) + nock(apiProxyUrl) + .persist() + .get(new RegExp(`/cosmos/staking/v1beta1/delegators/${address}/redelegations.*`)) .reply(200, {- nock(apiProxyUrl) - .persist() - .get(`/cosmos/distribution/v1beta1/delegators/${address}/rewards`) + nock(apiProxyUrl) + .persist() + .get(new RegExp(`/cosmos/distribution/v1beta1/delegators/${address}/rewards`)) .reply(200, {- nock(apiProxyUrl) - .persist() - .get( - `/akash/deployment/v1beta3/deployments/list?filters.owner=${address}&pagination.limit=10&pagination.offset=0&pagination.count_total=true&pagination.reverse=false` - ) + nock(apiProxyUrl) + .persist() + .get( + new RegExp(`/akash/deployment/v1beta3/deployments/list\\?filters\\.owner=${address}.*`) + ) .reply(200, {- nock(apiProxyUrl) - .persist() - .get(`/akash/market/v1beta4/leases/list?filters.owner=${address}&filters.state=active`) + nock(apiProxyUrl) + .persist() + .get(new RegExp(`/akash/market/v1beta4/leases/list\\?filters\\.owner=${address}.*`)) .reply(200, {Optional: Add nock.disableNetConnect() in a top-level test setup to ensure no unexpected external calls slip through.
Also applies to: 241-263, 264-293, 294-323, 379-438, 440-462
apps/api/test/functional/validators.spec.ts (1)
25-122: Use regex for GET matchers to tolerate query param ordering/changesThe .get() paths with query strings are exact string matches. Switching to RegExp reduces brittleness if the HTTP client changes query param ordering or adds defaults.
Suggested diffs:
- nock(apiProxyUrl) - .persist() - .get(`/cosmos/staking/v1beta1/validators?status=BOND_STATUS_BONDED&pagination.limit=1000`) + nock(apiProxyUrl) + .persist() + .get(new RegExp(`/cosmos/staking/v1beta1/validators\\?status=BOND_STATUS_BONDED.*`)) .reply(200, {- nock(apiProxyUrl) - .persist() - .get(`/cosmos/staking/v1beta1/validators/${validators[0].operatorAddress}`) + nock(apiProxyUrl) + .persist() + .get(new RegExp(`/cosmos/staking/v1beta1/validators/${validators[0].operatorAddress}`)) .reply(200, {- nock(apiProxyUrl).persist().get(`/cosmos/staking/v1beta1/validators/${validators[1].operatorAddress}`).reply(404); + nock(apiProxyUrl) + .persist() + .get(new RegExp(`/cosmos/staking/v1beta1/validators/${validators[1].operatorAddress}`)) + .reply(404);Also applies to: 124-157, 159-159
apps/api/test/functional/proposals.spec.ts (1)
98-224: All Nock hosts updated to the proxy — LGTM.
The mocked endpoints correctly point to apiProxyUrl, and .persist() is paired with nock.cleanAll() in afterAll, which prevents leakiness across test files.Optional: Consider adding nock.disableNetConnect() for these tests to hard-fail unintended outbound calls and ensure all network interactions are mocked.
Also applies to: 225-262, 263-274, 275-276
apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts (2)
86-92: Good pivot to ChainConfigService; consider widening endpoint type for future-proofing.Nice move to centralize the RPC URL via ChainConfigService and fast-fail when missing. One small improvement: align ConnectWithSignerFn’s endpoint type with SyncSigningStargateClient.connectWithSigner, which accepts string | HttpEndpoint. This avoids unnecessary narrowing and future shims.
Recommended type update (outside the selected lines):
// Align with SyncSigningStargateClient.connectWithSigner signature import type { HttpEndpoint } from "@cosmjs/tendermint-rpc"; type ConnectWithSignerFn = (endpoint: string | HttpEndpoint, wallet: Wallet, options: { registry: Registry }) => Promise<SyncSigningStargateClient>;
57-57: Ensure numeric timeout value for DataLoader batch schedule.Depending on BillingConfigService.get typing, this can be a string. Make the conversion explicit to avoid implicit coercions and subtle type issues.
- { cache: false, batchScheduleFn: callback => setTimeout(callback, this.config.get("WALLET_BATCHING_INTERVAL_MS")) } + { cache: false, batchScheduleFn: callback => setTimeout(callback, Number(this.config.get("WALLET_BATCHING_INTERVAL_MS"))) }apps/api/src/chain/services/block-http/block-http.service.ts (1)
9-14: Rename parameter for clarity.The class and parameter are both named “BlockHttpService,” which is easy to misread. Rename the constructor param to blockHttpServiceWrapper for readability.
- constructor(private readonly blockHttpService: BlockHttpServiceWrapper) {} + constructor(private readonly blockHttpServiceWrapper: BlockHttpServiceWrapper) {} @Memoize({ ttlInSeconds: averageBlockTime }) async getCurrentHeight() { - return await this.blockHttpService.getCurrentHeight(); + return await this.blockHttpServiceWrapper.getCurrentHeight(); }apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.spec.ts (2)
177-177: Align setup() signature with test guidelines (accept a single param with inline type).The repo’s test conventions specify a setup function that accepts a single parameter with inline typing. Add an optional args object for flexibility (e.g., customizable rpcUrl) while keeping current calls working.
- function setup() { + function setup({ rpcUrl = "http://localhost:26657" }: { rpcUrl?: string } = {}) {
202-204: Drive setup() via args and remove hard-coded RPC.This complements the setup signature change and improves test flexibility.
- const mockChainConfigService = mock<ChainConfigService>(); - mockChainConfigService.getBaseRpcUrl.mockReturnValue("http://localhost:26657"); + const mockChainConfigService = mock<ChainConfigService>(); + mockChainConfigService.getBaseRpcUrl.mockReturnValue(rpcUrl);apps/api/src/billing/services/chain-error/chain-error.service.ts (1)
108-114: Guard against empty USDC denom list in regex.If USDC_IBC_DENOMS is empty, the alternation becomes
(uakt|)which may lead to unexpected matches. Safe-guard by building the alternation only when there are USDC denoms.- const usdcDenoms = Object.values(this.billingConfigService.get("USDC_IBC_DENOMS")) - .map(denom => denom.replace(/\//g, "\\/")) - .join("|"); - - const pattern = new RegExp(`(\\d+)(uakt|${usdcDenoms}) is smaller than (\\d+)\\2`); + const usdcDenomsList = Object.values(this.billingConfigService.get("USDC_IBC_DENOMS")); + const usdcAlt = usdcDenomsList.length + ? `|${usdcDenomsList.map(denom => denom.replace(/\//g, "\\/")).join("|")}` + : ""; + const pattern = new RegExp(`(\\d+)(uakt${usdcAlt}) is smaller than (\\d+)\\2`);apps/notifications/src/modules/chain/providers/tendermint-client/tendermint-client.provider.ts (1)
8-10: Remove redundant await and improve readability in the async factory.No need to await the returned Promise from
Client.connect. Also, naming the resolved URL improves clarity.- async (rpcUrlResolver: RpcUrlResolverService): Promise<Tendermint34Client> => { - return await Client.connect(await rpcUrlResolver.getRpcUrl()); - }; + async (rpcUrlResolver: RpcUrlResolverService): Promise<Tendermint34Client> => { + const url = await rpcUrlResolver.getRpcUrl(); + return Client.connect(url); + };apps/api/src/proposal/services/proposal/proposal.service.ts (3)
12-21: Use explicit radix when parsing integers.Explicitly pass base 10 to avoid accidental octal/hex parsing and improve clarity.
- const proposals = proposalsFromCosmos.map(x => ({ - id: parseInt(x.proposal_id), + const proposals = proposalsFromCosmos.map(x => ({ + id: Number.parseInt(x.proposal_id, 10), title: x.content.title, status: x.status, submitTime: x.submit_time, votingStartTime: x.voting_start_time, votingEndTime: x.voting_end_time, - totalDeposit: parseInt(x.total_deposit[0]?.amount || "0") + totalDeposit: Number.parseInt(x.total_deposit[0]?.amount ?? "0", 10) }));
32-49: Avoid implicit any for tally and add radix 10 to all parseInt calls.Typing
tallyprevents implicit any and improves maintainability. Also, pass radix 10 for clarity. Note: if values can exceed JS safe integer range, consider BigInt/BN in a follow-up.- let tally = null; + let tally: ProposalTally; if (proposalFromCosmos.status === "PROPOSAL_STATUS_VOTING_PERIOD") { - const tallyFromCosmos = await this.cosmosHttpServiceWrapper.getProposalTally(id); + const tallyFromCosmos = await this.cosmosHttpServiceWrapper.getProposalTally(id); tally = { - yes: parseInt(tallyFromCosmos.yes) || 0, - abstain: parseInt(tallyFromCosmos.abstain) || 0, - no: parseInt(tallyFromCosmos.no) || 0, - noWithVeto: parseInt(tallyFromCosmos.no_with_veto) || 0 + yes: Number.parseInt(tallyFromCosmos.yes ?? "0", 10) || 0, + abstain: Number.parseInt(tallyFromCosmos.abstain ?? "0", 10) || 0, + no: Number.parseInt(tallyFromCosmos.no ?? "0", 10) || 0, + noWithVeto: Number.parseInt(tallyFromCosmos.no_with_veto ?? "0", 10) || 0 }; } else { tally = { - yes: parseInt(proposalFromCosmos.final_tally_result?.yes || "0") || 0, - abstain: parseInt(proposalFromCosmos.final_tally_result?.abstain || "0") || 0, - no: parseInt(proposalFromCosmos.final_tally_result?.no || "0") || 0, - noWithVeto: parseInt(proposalFromCosmos.final_tally_result?.no_with_veto || "0") || 0 + yes: Number.parseInt(proposalFromCosmos.final_tally_result?.yes ?? "0", 10) || 0, + abstain: Number.parseInt(proposalFromCosmos.final_tally_result?.abstain ?? "0", 10) || 0, + no: Number.parseInt(proposalFromCosmos.final_tally_result?.no ?? "0", 10) || 0, + noWithVeto: Number.parseInt(proposalFromCosmos.final_tally_result?.no_with_veto ?? "0", 10) || 0 }; }Add this lightweight type somewhere near the class (outside the changed lines):
type ProposalTally = { yes: number; abstain: number; no: number; noWithVeto: number; };
51-60: Apply radix 10 consistently in getProposalById response.For consistency with the above change and to avoid subtle parsing pitfalls.
- id: parseInt(proposalFromCosmos.proposal_id), + id: Number.parseInt(proposalFromCosmos.proposal_id, 10), title: proposalFromCosmos.content.title, description: proposalFromCosmos.content.description, status: proposalFromCosmos.status, submitTime: proposalFromCosmos.submit_time, votingStartTime: proposalFromCosmos.voting_start_time, votingEndTime: proposalFromCosmos.voting_end_time, - totalDeposit: parseInt(proposalFromCosmos.total_deposit[0]?.amount || "0"), + totalDeposit: Number.parseInt(proposalFromCosmos.total_deposit[0]?.amount ?? "0", 10), tally: { ...tally, total: tally.yes + tally.abstain + tally.no + tally.noWithVeto },apps/api/src/address/services/address/address.service.ts (2)
23-27: Concurrent wrapper calls: OK; consider resilient partial fetch with Promise.allSettledCurrent behavior fails the entire request if any upstream call fails. If you want partial data when some endpoints are down, switch to Promise.allSettled and default problematic parts.
34-38: Defensive access on commission response to avoid runtime TypeErrorsDepending on how the wrapper normalizes responses, the nested commission path could be undefined.
Apply this diff to guard access:
- const commissionData = await this.cosmosHttpServiceWrapper.getDistributionValidatorsCommissionByAddress(validatorFromDb.operatorAddress); - const coin = commissionData.commission.commission.find(x => x.denom === "uakt"); + const commissionData = await this.cosmosHttpServiceWrapper.getDistributionValidatorsCommissionByAddress(validatorFromDb.operatorAddress); + const coin = commissionData?.commission?.commission?.find((x: { denom: string; amount: string }) => x.denom === "uakt"); commission = coin ? parseFloat(coin.amount) : 0;apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts (1)
122-130: Group allowance lookups to reduce latencyThese two awaits are independent and can be parallelized for faster response time in revokeAll.
Refactor like:
- if (await this.authzHttpServiceWrapper.hasFeeAllowance(params.granter, params.grantee)) { + const [hasFeeAllowance, hasDeploymentGrant] = await Promise.all([ + this.authzHttpServiceWrapper.hasFeeAllowance(params.granter, params.grantee), + this.authzHttpServiceWrapper.hasDepositDeploymentGrant(params.granter, params.grantee) + ]); + + if (hasFeeAllowance) { revokeSummary.feeAllowance = true; messages.push(this.rpcMessageService.getRevokeAllowanceMsg(params)); } - if (await this.authzHttpServiceWrapper.hasDepositDeploymentGrant(params.granter, params.grantee)) { + if (hasDeploymentGrant) { revokeSummary.deploymentGrant = true; messages.push(this.rpcMessageService.getRevokeDepositDeploymentGrantMsg(params)); }apps/notifications/src/modules/chain/providers/stargate-client/stargate-client.provider.ts (1)
8-10: Nit: drop redundant await in factoryReturning the promise from Client.connect is sufficient.
Apply this diff:
- async (rpcUrlResolver: RpcUrlResolverService): Promise<StargateClient> => { - return await Client.connect(await rpcUrlResolver.getRpcUrl()); - }; + async (rpcUrlResolver: RpcUrlResolverService): Promise<StargateClient> => { + return Client.connect(await rpcUrlResolver.getRpcUrl()); + };apps/notifications/src/modules/chain/services/feature-flags/feature-flags.service.ts (1)
11-18: Avoid magic strings; use the FeatureFlags enum for consistencyThis ensures refactor safety and parity with other modules (e.g., RpcUrlResolverService).
Apply this diff:
-import type { FeatureFlagValue } from "./feature-flags"; +import { FeatureFlags, type FeatureFlagValue } from "./feature-flags"; @@ isEnabled(featureFlag: FeatureFlagValue): boolean { switch (featureFlag) { - case "use_proxy_urls": + case FeatureFlags.USE_PROXY_URLS: return this.configService.get("chain.USE_PROXY_URLS") === "true"; default: return false; } }apps/api/src/dashboard/services/stats/stats.service.ts (1)
273-301: Mind numeric precision when parsing Cosmos token amountsparseInt/parseFloat on large uakt supply/bonded values can exceed JS safe integer, causing precision loss. Consider using BigInt or a decimal lib for on-chain amounts, and only convert to number when rendering or after scaling down.
For example:
// sketch: handle amounts as bigints/decimals const bonded = BigInt(stakingPool.bonded_tokens); const supplyUakt = BigInt((supply.find(x => x.denom === "uakt")?.amount) ?? "0"); // ... compute ratios using a decimal library if you need APR with inflation/communityTax floatsapps/provider-proxy/src/services/feature-flags/feature-flags.service.ts (1)
3-4: Consider injecting env for testabilityReading process.env directly is fine here, but injecting an env reader (or passing defaults via constructor) will make unit testing and future extension smoother without relying on global state.
Example direction:
- Accept a getEnv(key) function in the constructor, defaulting to k => process.env[k].
apps/provider-proxy/src/container.ts (1)
4-4: Avoid deep import of generated data from package internalsImporting from @akashnetwork/net/src/... couples you to package internals. Prefer re-exporting netConfigData from @akashnetwork/net and importing from the package root to reduce breakage risk when internals change.
Proposed direction (package-level change required):
- Re-export netConfigData in @akashnetwork/net index.
- Then update this import to: import { netConfigData } from "@akashnetwork/net";
apps/api/src/billing/services/balances/balances.service.ts (2)
56-63: Add radix to parseInt for clarityExplicitly specify base 10 in parseInt to avoid surprises and quiet linters.
- return feeAllowance.allowance.spend_limit.reduce((acc, { denom, amount }) => (denom === "uakt" ? acc + parseInt(amount) : acc), 0); + return feeAllowance.allowance.spend_limit.reduce( + (acc, { denom, amount }) => (denom === "uakt" ? acc + parseInt(amount, 10) : acc), + 0 + );
66-77: Also specify radix when parsing deployment limitSame reasoning as above.
- return parseInt(depositDeploymentGrant.authorization.spend_limit.amount); + return parseInt(depositDeploymentGrant.authorization.spend_limit.amount, 10);packages/net/src/NetConfig/NetConfig.ts (1)
24-26: Confirm mainnet-only proxy gating is intentionalProxy usage is hard-gated to network === "mainnet". If you intend to enable proxying for sandbox/testnet (e.g., in CI or staging), consider removing the network check.
If you want to allow proxies on any supported network, apply:
- if (this.options.useProxyUrls && this.options.proxyApiUrl && network === "mainnet") { + if (this.options.useProxyUrls && this.options.proxyApiUrl) { return this.options.proxyApiUrl; }- if (this.options.useProxyUrls && this.options.proxyRpcUrl && network === "mainnet") { + if (this.options.useProxyUrls && this.options.proxyRpcUrl) { return this.options.proxyRpcUrl; }Also applies to: 46-48
apps/api/src/validator/services/validator/validator.service.ts (1)
52-60: Rank computation: handle missing validator gracefullyIf the address is not in the current bonded set, findIndex returns -1 and you return rank 0. Consider returning null or omitting rank in that case.
If acceptable in your API, adjust as:
- const validatorRank = validatorsResponse.validators + const validatorRankIndex = validatorsResponse.validators .map(x => { return { votingPower: parseInt(x.tokens), address: x.operator_address }; }) .sort((a, b) => b.votingPower - a.votingPower) .findIndex(x => x.address === address); ... - rank: validatorRank + 1 + rank: validatorRankIndex === -1 ? null : validatorRankIndex + 1If the response schema requires a number, confirm whether 0 or -1 is the preferred sentinel.
Also applies to: 74-75
apps/api/src/chain/services/block-http/block-http.service.spec.ts (1)
8-16: Align setup() with testing guidelines (signature and placement)
- Do not annotate the return type of setup.
- Accept a single parameter with inline type definition (even if unused).
- Place setup at the bottom of the root describe block.
Minimal signature fix:
- function setup(): { - blockHttpServiceWrapper: MockProxy<BlockHttpServiceWrapper>; - service: BlockHttpServiceClass; - } { + function setup(_: { } = {}) { const blockHttpServiceWrapper = mock<BlockHttpServiceWrapper>(); const service = new BlockHttpServiceClass(blockHttpServiceWrapper); - return { blockHttpServiceWrapper, service }; + return { blockHttpServiceWrapper, service }; }And move the setup() function definition to the bottom of the outermost describe("BlockHttpService", ...).
apps/api/src/core/services/http-service-wrapper/http-service-wrapper.ts (1)
64-69: Consider RPC-base wrappers if any SDK client needs RPC hostAll wrappers currently use getBaseAPIUrl(). If any http-sdk client should point to the RPC base (e.g., WebSocket or Tendermint RPC consumers), provide an RPC-specific wrapper using chainConfigService.getBaseRpcUrl().
apps/api/src/provider/services/provider/provider.service.ts (1)
40-48: Wrapper usage for provider fetch: ok; consider network-error handlingIf getProvider throws (network/5xx), this will bubble as a generic error later. If desired, explicitly differentiate "not found" (null) from transport failures here.
apps/api/src/core/services/chain-config/chain-config.service.ts (2)
9-10: Validate NETWORK and avoid unchecked casts; cache a narrowed network valueCasting env.NETWORK to SupportedChainNetworks without validation can hide misconfigurations and cause runtime errors in netConfig lookups. Cache a validated, narrowed network in the service and use it consistently.
Apply this diff:
export class ChainConfigService { private readonly netConfig: ReturnType<typeof createNetConfig>; + private readonly network: SupportedChainNetworks; constructor(private readonly featureFlagsService: FeatureFlagsService) { this.netConfig = createNetConfig({ useProxyUrls: this.featureFlagsService.isEnabled("use_proxy_urls"), proxyApiUrl: env.PROXY_API_URL, proxyRpcUrl: env.PROXY_RPC_URL }); + const candidates = this.netConfig.getSupportedNetworks(); + const maybeNetwork = env.NETWORK as string; + if (!candidates.includes(maybeNetwork as SupportedChainNetworks)) { + throw new Error(`Unsupported NETWORK "${maybeNetwork}". Supported: ${candidates.join(", ")}`); + } + this.network = maybeNetwork as SupportedChainNetworks; } getBaseAPIUrl(): string { - return this.netConfig.getBaseAPIUrl(env.NETWORK as SupportedChainNetworks); + return this.netConfig.getBaseAPIUrl(this.network); } getBaseRpcUrl(): string { - return this.netConfig.getBaseRpcUrl(env.NETWORK as SupportedChainNetworks); + return this.netConfig.getBaseRpcUrl(this.network); }Also applies to: 11-17, 19-25
19-25: Naming consistency nit: API vs ApiMethod name getBaseAPIUrl uses “API” while getBaseRpcUrl uses “Rpc”. Consider getBaseApiUrl for consistency with the broader codebase naming (e.g., Stats Web).
apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts (3)
32-35: Redundant null-check on rpcUrl if ChainConfigService guarantees a stringIf ChainConfigService.getBaseRpcUrl() cannot return undefined (current signature is string), this check can be removed. If it can be undefined for unsupported networks, prefer validating and narrowing the network in ChainConfigService instead (see earlier comment).
115-116: Switch to BidHttpServiceWrapper.list is correctCall site updated cleanly to the wrapper abstraction. Consider adding a small guard/log if bids retrieval fails, to avoid blocking the close path.
- const bids = await this.bidHttpServiceWrapper.list(walletAddress, dseq); + let bids = []; + try { + bids = await this.bidHttpServiceWrapper.list(walletAddress, dseq); + } catch (e) { + this.logger.warn({ event: "BIDS_LIST_FAILED", dseq, error: (e as Error).message }); + }
182-191: Harden latest-height fetch: add retry and timeoutDirect axios.get without retry/timeout can be flaky. Either reuse an existing resilient HTTP client or add a lightweight retry with backoff here.
Apply this diff:
- const apiNodeUrl = this.chainConfigService.getBaseAPIUrl(); - const response = await axios.get(`${apiNodeUrl}/blocks/latest`); - - const height = parseInt(response.data.block.header.height); + const apiNodeUrl = this.chainConfigService.getBaseAPIUrl(); + const maxAttempts = 3; + let height = NaN; + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + try { + const response = await axios.get(`${apiNodeUrl}/blocks/latest`, { timeout: 10_000 }); + height = parseInt(response.data?.block?.header?.height); + if (!isNaN(height)) break; + this.logger.warn({ event: "LATEST_BLOCK_SHAPE_UNEXPECTED", attempt }); + } catch (err) { + if (attempt === maxAttempts) throw err; + await sleep(500 * attempt); + } + }apps/stats-web/src/services/api-url/api-url.service.ts (1)
16-25: Add an explicit fallback for unsupported config shapesCurrently, if neither server nor browser shape is matched, the function falls through without returning. Add an explicit error to surface misconfiguration early.
getBaseApiUrlFor(network: NetworkId, useProxyUrls = false) { // Only use proxy URLs for mainnet if (useProxyUrls && network === MAINNET_ID) { const proxyUrl = "PROXY_API_URL" in this.config ? this.config.PROXY_API_URL : (this.config as Pick<BrowserEnvConfig, "NEXT_PUBLIC_PROXY_API_URL">).NEXT_PUBLIC_PROXY_API_URL; return proxyUrl; } if ("BASE_API_MAINNET_URL" in this.config) { @@ } } + + throw new Error("ApiUrlService: Unsupported environment config shape"); }apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (2)
87-90: Typo: rename deploymentReponse -> deploymentResponseMinor readability fix; avoids confusion.
- const deploymentReponse = await this.deploymentHttpServiceWrapper.loadDeploymentList(owner, "active", pagination); - const deployments = deploymentReponse.deployments; - const total = parseInt(deploymentReponse.pagination.total, 10); + const deploymentResponse = await this.deploymentHttpServiceWrapper.loadDeploymentList(owner, "active", pagination); + const deployments = deploymentResponse.deployments; + const total = parseInt(deploymentResponse.pagination.total, 10);
186-193: Consistent error semanticsGood normalization of “deployment not found” to null while surfacing other errors. Consider using InternalServerError for non-404 cases to align with findByOwnerAndDseq.
- } else { - throw new Error(deploymentData.message); - } + } else { + throw new InternalServerError(deploymentData.message); + }apps/api/src/billing/services/financial-stats/financial-stats.service.ts (1)
58-61: Add retry/timeout when fetching community pool, or route via a wrapperDirect axios.get can be brittle. Consider using the same resilient client pattern (wrapper) or add a lightweight retry with timeout here.
Apply this diff:
- const apiNodeUrl = this.chainConfigService.getBaseAPIUrl(); - const communityPoolData = await axios.get<CosmosDistributionCommunityPoolResponse>(`${apiNodeUrl}/cosmos/distribution/v1beta1/community_pool`); - return parseFloat(communityPoolData.data.pool.find(x => x.denom === USDC_IBC_DENOMS.mainnetId)?.amount || "0"); + const apiNodeUrl = this.chainConfigService.getBaseAPIUrl(); + const url = `${apiNodeUrl}/cosmos/distribution/v1beta1/community_pool`; + const maxAttempts = 3; + let data: CosmosDistributionCommunityPoolResponse | undefined; + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + try { + const res = await axios.get<CosmosDistributionCommunityPoolResponse>(url, { timeout: 10_000 }); + data = res.data; + break; + } catch (e) { + if (attempt === maxAttempts) throw e; + await new Promise(r => setTimeout(r, 300 * attempt)); + } + } + return parseFloat(data?.pool.find(x => x.denom === USDC_IBC_DENOMS.mainnetId)?.amount || "0");apps/api/src/billing/services/chain-error/chain-error.service.spec.ts (2)
82-82: Typo: “IBС” uses Cyrillic ‘С’—change to ASCII “IBC”This can confuse searches and readers.
- it("returns 400 for an unsupported IBС denom", async () => { + it("returns 400 for an unsupported IBC denom", async () => {
123-127: Avoid casting to jest.Mock when stubbing BillingConfigService.getLeverage jest-mock-extended’s typed mocks to remove the cast for cleaner types.
For example (if the method signature allows), prefer:
- (billingConfigService.get as jest.Mock).mockImplementation((key: string) => { + billingConfigService.get.mockImplementation((key: string) => { if (key === "USDC_IBC_DENOMS") return USDC_IBC_DENOMS; if (key === "DEPLOYMENT_GRANT_DENOM") return "uakt"; return undefined; });If generics/overloads block this, consider narrowing via a small wrapper function or using jest-mock-extended’s calledWith API if available in your version:
billingConfigService.get.calledWith("USDC_IBC_DENOMS").mockReturnValue(USDC_IBC_DENOMS); billingConfigService.get.calledWith("DEPLOYMENT_GRANT_DENOM").mockReturnValue("uakt");apps/api/test/functional/deployments.spec.ts (4)
172-173: Duplicate nock for leases list (active) — remove oneTwo identical interceptors for the same path can mask unintended extra calls and complicate debugging.
- nock(apiProxyUrl).persist().get(`/akash/market/${betaTypeVersionMarket}/leases/list?filters.owner=${address}&filters.state=active`).reply(200, { leases }); - nock(apiProxyUrl).persist().get(`/akash/market/${betaTypeVersionMarket}/leases/list?filters.owner=${address}&filters.state=active`).reply(200, { leases }); + nock(apiProxyUrl).persist().get(`/akash/market/${betaTypeVersionMarket}/leases/list?filters.owner=${address}&filters.state=active`).reply(200, { leases });
266-269: Avoid hardcoding API version in regex; use betaTypeVersionHardcoding v1beta3 in the regex introduces drift if the version changes. Use the imported betaTypeVersion for all occurrences.
- nock(apiProxyUrl) - .persist() - .get(/\/akash\/deployment\/v1beta3\/deployments\/list\?.*/) + nock(apiProxyUrl) + .persist() + .get(new RegExp(`/akash/deployment/${betaTypeVersion}/deployments/list\\?.*`)) .reply(200, { deployments, pagination: { total: deployments.length, next_key: null } });- nock(apiProxyUrl) - .persist() - .get(/\/akash\/deployment\/v1beta3\/deployments\/list\?.*/) + nock(apiProxyUrl) + .persist() + .get(new RegExp(`/akash/deployment/${betaTypeVersion}/deployments/list\\?.*`)) .reply(200, { deployments: deployments.slice(0, 1), pagination: { total: deployments.length, next_key: null } });- nock(apiProxyUrl) - .persist() - .get(/\/akash\/deployment\/v1beta3\/deployments\/list\?.*/) + nock(apiProxyUrl) + .persist() + .get(new RegExp(`/akash/deployment/${betaTypeVersion}/deployments/list\\?.*`)) .reply(200, { deployments, pagination: { total: deployments.length, next_key: null } });Also applies to: 305-308, 366-369
96-99: afterAll duplicates cleanup already done in afterEachYou already restore mocks and clean nock in afterEach. The afterAll duplication is redundant.
- afterAll(async () => { - jest.restoreAllMocks(); - nock.cleanAll(); - }); + // afterAll cleanup is redundant; afterEach handles it
45-89: Consider replacing beforeEach scaffolding with a local setup() to align with test guidelinesCurrent tests use beforeEach, but the repository guidelines for *.spec.ts prefer a setup() factory at the bottom of the root describe. It would make the test wiring more explicit and reduce hidden shared state.
apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts
Outdated
Show resolved
Hide resolved
apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts
Outdated
Show resolved
Hide resolved
apps/notifications/src/modules/chain/services/rpc-url-resolver/rpc-url-resolver.service.ts
Outdated
Show resolved
Hide resolved
| constructor(options: NetConfigOptions = {}) { | ||
| this.options = { | ||
| useProxyUrls: false, | ||
| proxyApiUrl: "https://rpc.akt.dev/rest", | ||
| proxyRpcUrl: "https://rpc.akt.dev/rpc", | ||
| ...options | ||
| }; | ||
| } |
There was a problem hiding this comment.
Avoid embedding path segments in proxy defaults; this breaks URL resolution for mixed absolute/relative SDK endpoints
Setting proxyApiUrl to "https://rpc.akt.dev/rest" and proxyRpcUrl to "https://rpc.akt.dev/rpc" introduces inconsistent behavior because many http-sdk endpoints start with a leading slash (e.g., "/cosmos/..."), which will drop the "/rest" path component. Others (e.g., "blocks/latest") are relative and will keep it—leading to divergent base paths. Remove these opinionated defaults (or use a bare origin) to avoid subtle routing bugs.
Apply this diff to neutralize the risky defaults:
constructor(options: NetConfigOptions = {}) {
this.options = {
useProxyUrls: false,
- proxyApiUrl: "https://rpc.akt.dev/rest",
- proxyRpcUrl: "https://rpc.akt.dev/rpc",
+ proxyApiUrl: undefined,
+ proxyRpcUrl: undefined,
...options
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| constructor(options: NetConfigOptions = {}) { | |
| this.options = { | |
| useProxyUrls: false, | |
| proxyApiUrl: "https://rpc.akt.dev/rest", | |
| proxyRpcUrl: "https://rpc.akt.dev/rpc", | |
| ...options | |
| }; | |
| } | |
| constructor(options: NetConfigOptions = {}) { | |
| this.options = { | |
| useProxyUrls: false, | |
| proxyApiUrl: undefined, | |
| proxyRpcUrl: undefined, | |
| ...options | |
| }; | |
| } |
🤖 Prompt for AI Agents
In packages/net/src/NetConfig/NetConfig.ts around lines 14 to 21, the
constructor sets proxyApiUrl and proxyRpcUrl to origins that include path
segments ("/rest" and "/rpc"), which causes inconsistent URL resolution when SDK
endpoints are absolute (leading slash) vs relative; remove the opinionated path
segments and default these values to neutral origins (e.g.,
"https://rpc.akt.dev") or leave them undefined so callers supply full URLs;
update the options default to use bare origins or undefined for
proxyApiUrl/proxyRpcUrl and ensure any code that consumes these values treats
them as origins and joins paths safely.
| if (this.options.useProxyUrls && this.options.proxyApiUrl && network === "mainnet") { | ||
| return this.options.proxyApiUrl; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify baseURL semantics across SDKs to avoid path dropping with leading slashes
Many http-sdk endpoints are prefixed with "/", which causes axios to resolve against the origin and drop any path segments in baseURL (like "/rest"). Ensure PROXY_API_URL is an origin (no path), or standardize SDK endpoint paths to be relative consistently.
Run this script to list endpoints with and without leading slashes in http-sdk, confirming the risk:
🏁 Script executed:
#!/bin/bash
# Show http-sdk GET calls starting with a leading slash (absolute) vs without (relative)
echo "=== Absolute-path GETs (start with /) ==="
rg -nP --type=ts -C1 'get<[^>]*>\(\s*[`"\']/'
echo
echo "=== Relative-path GETs (no leading /) ==="
rg -nP --type=ts -C1 'get<[^>]*>\(\s*[`"\'][^/]'Length of output: 209
I encountered a shell-quoting issue in the last script. Let’s retry with properly escaped patterns to classify absolute vs. relative GET endpoints:
🏁 Script executed:
#!/bin/bash
# Show http-sdk GET calls starting with a leading slash (absolute) vs without (relative)
echo "=== Absolute-path GETs (start with /) ==="
rg -nP --type=ts -C1 "get<[^>]*>\\(\\s*['\"\`]/"
echo
echo "=== Relative-path GETs (no leading /) ==="
rg -nP --type=ts -C1 "get<[^>]*>\\(\\s*['\"\`][^/]"Length of output: 20443
Ensure consistent URL resolution for proxy API
Axios treats request URLs starting with “/” as absolute paths, dropping any path segments in baseURL (e.g. “/rest”). Since most http-sdk endpoints begin with “/”, returning a proxyApiUrl that includes a path—rather than a pure origin—will cause requests to miss that path segment.
Please address this by:
- In
packages/net/src/NetConfig/NetConfig.ts, validate or enforce thatoptions.proxyApiUrlis a bare origin (no path component), or strip any path before assigning it as the base URL. - Or refactor all endpoints in
packages/http-sdkto use relative paths (remove the leading “/”) so that abaseURLwith paths (like/rest) is correctly preserved.
🤖 Prompt for AI Agents
In packages/net/src/NetConfig/NetConfig.ts around lines 24-26, the code returns
options.proxyApiUrl directly which may include a path; because Axios treats URLs
starting with "/" as absolute and drops baseURL path segments, ensure the
proxyApiUrl used as a base is a bare origin: validate and strip any path
component from options.proxyApiUrl before returning (or throw/config error if it
contains a path), e.g. parse the URL and return only scheme://host[:port];
alternatively, if you prefer keeping pathful base URLs, document and refactor
packages/http-sdk endpoints to remove leading "/" so they are relative paths and
will preserve baseURL path segments.
baktun14
force-pushed
the
features/rpc-proxy
branch
from
6336b5a to
0c328a5
Compare
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (37.50%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1823 +/- ##
==========================================
- Coverage 42.93% 42.70% -0.23%
==========================================
Files 943 944 +1
Lines 26541 26451 -90
Branches 6956 6915 -41
==========================================
- Hits 11395 11297 -98
- Misses 13976 13983 +7
- Partials 1170 1171 +1
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/api/src/validator/services/validator/validator.service.ts (2)
41-50: Avoidanyin catch and use axios.isAxiosError to check for 404.Aligns with the repo’s TS guideline to avoid
anyand makes 404 handling precise.Apply this diff (requires the axios import suggested above):
- } catch (error: any) { - if (error.response?.status === 404) { + } catch (err: unknown) { + if (axios.isAxiosError(err) && err.response?.status === 404) { return null; } - throw error; + throw err; }
54-60: Update validator schema & service to safely handle unranked validatorsTo return
nullwhen a validator isn’t found (rather than defaulting to rank 0), you must update both the service logic and the API schema:• In
apps/api/src/validator/services/validator/validator.service.ts(around line 60):const validatorRank = validatorsResponse.validators .map(x => ({ votingPower: parseInt(x.tokens), address: x.operator_address })) .sort((a, b) => b.votingPower - a.votingPower) .findIndex(x => x.address === address); + const computedRank = validatorRank === -1 ? null : validatorRank + 1; … - rank: validatorRank + 1 + rank: computedRank• In
apps/api/src/validator/http-schemas/validator.schema.ts(lines 14 and 39):- rank: z.number(), + rank: z.number().nullable(), … - rank: z.number() + rank: z.number().nullable()• Review any downstream types/clients to handle
rank: number | null.These schema changes are required to keep the API contract valid when returning
null.
🧹 Nitpick comments (23)
apps/api/src/billing/providers/signing-client.provider.ts (1)
14-21: ChainConfigService DI & Constructor Signature Verified
- ChainConfigService is decorated with
@singleton()in
apps/api/src/core/services/chain-config/chain-config.service.tsand will be auto-registered by tsyringe.BatchSigningClientService(in
apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts) now haschainConfigService: ChainConfigServicecorrectly inserted as the 5th constructor parameter, immediately before the client token.Optional: To avoid duplicating the six-argument factory across three registrations in
apps/api/src/billing/providers/signing-client.provider.ts, consider extracting a helper function (e.g.registerSigningClient) that takes the client token and wallet token and callscontainer.register(...)with the shared factory logic.apps/api/src/chain/services/block-http/block-http.service.spec.ts (2)
4-5: Avoid deep import; consider the services barrel exportIf BlockHttpServiceWrapper is re-exported from the core services index, prefer importing from the barrel to reduce path churn.
-import type { BlockHttpServiceWrapper } from "@src/core/services/http-service-wrapper/http-service-wrapper"; +import type { BlockHttpServiceWrapper } from "@src/core/services";
7-17: Minor test hygiene: use mockResolvedValueOnce and assert call countKeeps the stub scoped to this test and strengthens the assertion.
- blockHttpServiceWrapper.getCurrentHeight.mockResolvedValue(12345); + blockHttpServiceWrapper.getCurrentHeight.mockResolvedValueOnce(12345); @@ - expect(blockHttpServiceWrapper.getCurrentHeight).toHaveBeenCalled(); + expect(blockHttpServiceWrapper.getCurrentHeight).toHaveBeenCalledTimes(1);apps/stats-web/src/services/api-url/api-url.service.ts (1)
9-13: Env schema includes proxy keys as expected; optional type alias for readability
- Verified that both
PROXY_API_URLandNEXT_PUBLIC_PROXY_API_URLare defined inapps/stats-web/src/config/env-config.schema.ts.- Optional: extract the long union of
Picktypes into a local alias to improve readability without changing behavior:// at top of apps/stats-web/src/services/api-url/api-url.service.ts type ApiUrlConfig = | Pick<ServerEnvConfig, "BASE_API_MAINNET_URL" | "BASE_API_TESTNET_URL" | "BASE_API_SANDBOX_URL" | "PROXY_API_URL"> | Pick< BrowserEnvConfig, "NEXT_PUBLIC_BASE_API_TESTNET_URL" | "NEXT_PUBLIC_BASE_API_SANDBOX_URL" | "NEXT_PUBLIC_BASE_API_MAINNET_URL" | "NEXT_PUBLIC_PROXY_API_URL" >; @Injectable() export class ApiUrlService { constructor(private readonly config: ApiUrlConfig) {} // … }apps/api/src/address/services/address/address.service.ts (1)
35-35: Minor latency win: start DB reads in parallel with network callsYou can overlap DB fetches with the wrapper/tx requests to trim end-to-end latency. Commission still stays conditional on validatorFromDb, but allValidators and the validator lookup need not wait.
Apply this refactor within the method:
- const [balancesResponse, delegationsResponse, rewardsResponse, redelegationsResponse, latestTransactions] = await Promise.all([ - this.cosmosHttpServiceWrapper.getBankBalancesByAddress(address), - this.cosmosHttpServiceWrapper.getStakingDelegationsByAddress(address), - this.cosmosHttpServiceWrapper.getDistributionDelegatorsRewardsByAddress(address), - this.cosmosHttpServiceWrapper.getStakingDelegatorsRedelegationsByAddress(address), - this.transactionService.getTransactionsByAddress({ address, skip: 0, limit: 5 }) - ]); - - const allValidatorsFromDb = await this.validatorRepository.findAll(); - const validatorFromDb = await this.validatorRepository.findByAccountAddress(address); + const validatorsPromise = this.validatorRepository.findAll(); + const validatorByAddressPromise = this.validatorRepository.findByAccountAddress(address); + + const [ + balancesResponse, + delegationsResponse, + rewardsResponse, + redelegationsResponse, + latestTransactions, + allValidatorsFromDb, + validatorFromDb + ] = await Promise.all([ + this.cosmosHttpServiceWrapper.getBankBalancesByAddress(address), + this.cosmosHttpServiceWrapper.getStakingDelegationsByAddress(address), + this.cosmosHttpServiceWrapper.getDistributionDelegatorsRewardsByAddress(address), + this.cosmosHttpServiceWrapper.getStakingDelegatorsRedelegationsByAddress(address), + this.transactionService.getTransactionsByAddress({ address, skip: 0, limit: 5 }), + validatorsPromise, + validatorByAddressPromise + ]); let commission = 0; if (validatorFromDb?.operatorAddress) { const commissionData = await this.cosmosHttpServiceWrapper.getDistributionValidatorsCommissionByAddress(validatorFromDb.operatorAddress);apps/api/src/validator/services/validator/validator.service.ts (3)
28-35: Guard against division by zero when computing votingPowerRatio.If totalVotingPower is 0 (e.g., empty list or all zero tokens), the ratio becomes Infinity/NaN.
Apply this diff:
.map((x, i) => ({ ...x, - votingPowerRatio: x.votingPower / totalVotingPower, + votingPowerRatio: totalVotingPower > 0 ? x.votingPower / totalVotingPower : 0, rank: i + 1, keybaseAvatarUrl: avatarMap.get(x.operatorAddress) ?? null }));
64-65: Consistency: trim moniker like in list().list() trims moniker, but getByAddress() doesn’t. Normalize to avoid whitespace discrepancies.
Apply this diff:
- moniker: cosmosResponse.validator.description.moniker, + moniker: cosmosResponse.validator.description.moniker.trim(),
17-25: Optional: consider BigInt for large token counts.Validator token values can exceed Number.MAX_SAFE_INTEGER. For accurate sorting/summing on very large chains, consider using BigInt (and only convert to number for final ratios), or a big-number library. This is optional if approximate ratios suffice.
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (5)
87-90: Typo: renamedeploymentReponsetodeploymentResponseMinor readability nit.
- const deploymentReponse = await this.deploymentHttpServiceWrapper.loadDeploymentList(owner, "active", pagination); - const deployments = deploymentReponse.deployments; - const total = parseInt(deploymentReponse.pagination.total, 10); + const deploymentResponse = await this.deploymentHttpServiceWrapper.loadDeploymentList(owner, "active", pagination); + const deployments = deploymentResponse.deployments; + const total = parseInt(deploymentResponse.pagination.total, 10);
91-95: Concurrency of 100 may be too high; consider making it configurableFetching leases for each deployment at concurrency 100 risks API rate limiting and uneven load. Consider lowering (e.g., 25–50) or making this a config/env to tune per environment.
185-200: Avoidanyin catch and align error throwing withInternalServerError
- Coding guideline: avoid
any. Useunknownand narrow.- For non-404 “code” responses, throw
InternalServerError(consistent withfindByOwnerAndDseqabove).- } else { - throw new Error(deploymentData.message); - } + } else { + throw new InternalServerError(deploymentData.message); + } } - } catch (error: any) { - if (error.response?.status === 404) { + } catch (error: unknown) { + const status = (error as { response?: { status?: number } })?.response?.status; + if (status === 404) { return null; } throw error; }
183-241: Type mismatch:deploymentDatadeclared asRestAkashDeploymentInfoResponsebut later cast toDeploymentInfoto accessgroups
deploymentDatais used both as a REST-shaped response (deployment,escrow_account) and as an SDKDeploymentInfo(groups). The explicit cast at Line 240 hides potential shape mismatches and can lead to runtime errors if wrappers ever change.Two safe options:
- If
findByOwnerAndDseqtruly returns SDKDeploymentInfo-compatible data, update the variable type:
- Change declaration to a compatible intersection:
let deploymentData: (RestAkashDeploymentInfoResponse & DeploymentInfo) | null = null;- Then you can drop the
as DeploymentInfoat Line 240.- Or, add a type guard before using
groups:function hasGroups(x: unknown): x is DeploymentInfo { return !!x && typeof x === "object" && "groups" in x; } const groups = hasGroups(deploymentData) ? deploymentData.groups : []; const group = groups.find(g => g.group_id.gseq === x.lease.lease_id.gseq);Would you like me to refactor this method to introduce the type guard and remove the cast?
135-135: Minor: specify radix inparseIntfor consistencyOther places use base 10 (e.g., Line 90). Suggest aligning here too.
- count: parseInt(response.pagination.total), + count: parseInt(response.pagination.total, 10),apps/api/env/.env.functional.test (1)
42-43: Add missing trailing newline to satisfy dotenv-linterThe linter warns about “No blank line at the end of the file.” Add a trailing newline.
Apply this diff:
-PROXY_RPC_URL=https://rpc.sandbox-01.aksh.pw:443 +PROXY_RPC_URL=https://rpc.sandbox-01.aksh.pw:443 +apps/api/src/provider/services/provider/provider.service.ts (1)
69-75: Avoidanyin catch and preserve useful error infoTyping the catch param as any violates our TS guideline and risks losing error context. Also, passing a non-string (e.g., response.data object) to Error(message) becomes “[object Object]”.
Apply this diff:
- } catch (err: any) { - if (err.message?.includes("no lease for deployment") && i < this.MANIFEST_SEND_MAX_RETRIES) { + } catch (err: unknown) { + if (err instanceof Error && err.message.includes("no lease for deployment") && i < this.MANIFEST_SEND_MAX_RETRIES) { await delay(this.MANIFEST_SEND_RETRY_DELAY); continue; } - throw new Error(err?.response?.data || err); + const hasResponse = (e: unknown): e is { response?: { data?: unknown } } => + typeof e === "object" && e !== null && "response" in e; + const responseData = hasResponse(err) ? err.response?.data : undefined; + const message = + typeof responseData === "string" + ? responseData + : responseData !== undefined + ? JSON.stringify(responseData) + : err instanceof Error + ? err.message + : JSON.stringify(err); + throw new Error(message, { cause: err }); }apps/api/test/functional/lease-flow.spec.ts (1)
85-93: Prefer “setup” function over beforeEach per test guidelinesOur test standard uses a setup() function at the bottom of the root describe block instead of beforeEach and avoids shared state. Consider moving currentHeight generation and spies into setup() returning the configured test fixtures.
Example shape to refactor toward (illustrative, not a drop-in):
// at the bottom of the root describe("Lease Flow", ...) function setup({ height }: { height?: number } = {}) { const blockHttpService = container.resolve(BlockHttpServiceWrapper); const providerService = container.resolve(ProviderService); const current = height ?? faker.number.int({ min: 1_000_000, max: 10_000_000 }); jest.spyOn(blockHttpService, "getCurrentHeight").mockResolvedValue(current); jest.spyOn(providerService, "sendManifest").mockResolvedValue(true); jest.spyOn(providerService, "getLeaseStatus").mockResolvedValue(LeaseStatusSeeder.create()); return { blockHttpService, providerService, current }; }apps/api/src/core/providers/http-sdk.provider.ts (1)
4-4: Ensure NodeHttpService uses the proxy URL instead of the direct node API
nodeApiBasePathis currently sourced fromNODE_API_BASE_PATH, soNodeHttpServicewill bypassPROXY_API_URL. To align with our proxy strategy and keep a single source of truth:• In apps/api/src/core/providers/http-sdk.provider.ts
- import { nodeApiBasePath } from "@src/utils/constants"; + import { apiProxyUrl } from "@src/utils/constants"; - container.register(NodeHttpService, { useValue: new NodeHttpService({ baseURL: nodeApiBasePath }) }); + container.register(NodeHttpService, { useValue: new NodeHttpService({ baseURL: apiProxyUrl }) });Optional: you can also pull the base URL from
ChainConfigServiceto centralize URL resolution across all services.apps/api/test/functional/deployments.spec.ts (6)
124-174: Duplicate nock interceptor for active leasesYou register the same interceptor twice for
filters.state=active. Remove the duplicate to avoid confusion and accidental order/side-effects.Apply this within the block:
- nock(apiProxyUrl).persist().get(`/akash/market/${betaTypeVersionMarket}/leases/list?filters.owner=${address}&filters.state=active`).reply(200, { leases }); + // (duplicate removed) - nock(apiProxyUrl).persist().get(`/akash/market/${betaTypeVersionMarket}/leases/list?filters.owner=${address}&filters.state=active`).reply(200, { leases });
266-275: Avoid hardcoding version in regex; prefer constant + .query(true)The path uses a hardcoded
v1beta3. This becomes brittle ifbetaTypeVersionchanges. Use the constant and.query(true)to match any query string instead of a regex.- nock(apiProxyUrl) - .persist() - .get(/\/akash\/deployment\/v1beta3\/deployments\/list\?.*/) - .reply(200, { + nock(apiProxyUrl) + .persist() + .get(`/akash/deployment/${betaTypeVersion}/deployments/list`) + .query(true) + .reply(200, { deployments, pagination: { total: deployments.length, next_key: null } });
305-314: Repeat the version-constant + .query(true) improvement hereSame reasoning as above: remove hardcoded
v1beta3and rely onbetaTypeVersion.- nock(apiProxyUrl) - .persist() - .get(/\/akash\/deployment\/v1beta3\/deployments\/list\?.*/) - .reply(200, { + nock(apiProxyUrl) + .persist() + .get(`/akash/deployment/${betaTypeVersion}/deployments/list`) + .query(true) + .reply(200, { deployments: deployments.slice(0, 1), pagination: { total: deployments.length, next_key: null } });
366-375: Apply the same pattern: remove hardcoded version and use .query(true)Keep tests resilient to version changes.
- nock(apiProxyUrl) - .persist() - .get(/\/akash\/deployment\/v1beta3\/deployments\/list\?.*/) - .reply(200, { + nock(apiProxyUrl) + .persist() + .get(`/akash/deployment/${betaTypeVersion}/deployments/list`) + .query(true) + .reply(200, { deployments, pagination: { total: deployments.length, next_key: null } });
574-576: Describe text vs route mismatch (deposit endpoint)Block title says
POST /v1/deployments/{dseq}/deposit, but tests call/v1/deposit-deployment. Align the description to the actual route to avoid confusion.-describe("POST /v1/deployments/{dseq}/deposit", () => { +describe("POST /v1/deposit-deployment", () => {Also applies to: 589-599
45-90: Tests use beforeEach; repository guideline prefers a setup() functionPer repo guidelines for *.spec.ts files:
- Use a setup() function (at the bottom of the root describe) instead of beforeEach.
- setup() should create and return the object(s) under test, accept a single param with inline type, avoid shared state, and not declare an explicit return type.
- Prefer passing mocks as dependencies (e.g., via jest-mock-extended) rather than spying on container singletons, where feasible.
These are functional tests, so the benefit is mainly consistency and reduced hidden shared state.
Would you like me to propose a minimal refactor showing a setup() pattern for one of these describe blocks to use as a template?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (59)
apps/api/env/.env.functional.test(1 hunks)apps/api/env/.env.mainnet(0 hunks)apps/api/env/.env.sample(0 hunks)apps/api/env/.env.sandbox(0 hunks)apps/api/env/.env.unit.test(0 hunks)apps/api/src/address/services/address/address.service.ts(3 hunks)apps/api/src/bid/controllers/bid/bid.controller.ts(2 hunks)apps/api/src/billing/config/env.config.ts(0 hunks)apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.spec.ts(3 hunks)apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts(3 hunks)apps/api/src/billing/providers/signing-client.provider.ts(4 hunks)apps/api/src/billing/services/balances/balances.service.ts(4 hunks)apps/api/src/billing/services/chain-error/chain-error.service.spec.ts(3 hunks)apps/api/src/billing/services/chain-error/chain-error.service.ts(3 hunks)apps/api/src/billing/services/dedupe-signing-client/dedupe-signing-client.service.ts(3 hunks)apps/api/src/billing/services/financial-stats/financial-stats.service.ts(4 hunks)apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts(4 hunks)apps/api/src/chain/services/block-http/block-http.service.spec.ts(1 hunks)apps/api/src/chain/services/block-http/block-http.service.ts(1 hunks)apps/api/src/core/providers/http-sdk.provider.ts(1 hunks)apps/api/src/core/services/chain-config/chain-config.service.ts(1 hunks)apps/api/src/core/services/feature-flags/feature-flags.ts(1 hunks)apps/api/src/core/services/http-service-wrapper/http-service-wrapper.ts(1 hunks)apps/api/src/core/services/index.ts(1 hunks)apps/api/src/dashboard/services/stats/stats.service.ts(4 hunks)apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts(7 hunks)apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts(3 hunks)apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts(5 hunks)apps/api/src/proposal/services/proposal/proposal.service.ts(2 hunks)apps/api/src/provider/services/provider/provider.service.ts(4 hunks)apps/api/src/utils/constants.ts(1 hunks)apps/api/src/utils/env.ts(1 hunks)apps/api/src/validator/services/validator/validator.service.ts(3 hunks)apps/api/test/functional/addresses.spec.ts(8 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(2 hunks)apps/api/test/functional/deployments.spec.ts(8 hunks)apps/api/test/functional/lease-flow.spec.ts(2 hunks)apps/api/test/functional/proposals.spec.ts(5 hunks)apps/api/test/functional/stale-anonymous-users-cleanup.spec.ts(2 hunks)apps/api/test/functional/start-trial.spec.ts(2 hunks)apps/api/test/functional/validators.spec.ts(4 hunks)apps/notifications/src/modules/chain/chain.module.ts(2 hunks)apps/notifications/src/modules/chain/config/env.config.ts(1 hunks)apps/notifications/src/modules/chain/providers/stargate-client/stargate-client.provider.ts(1 hunks)apps/notifications/src/modules/chain/providers/tendermint-client/tendermint-client.provider.ts(1 hunks)apps/notifications/src/modules/chain/services/feature-flags/feature-flags.service.ts(1 hunks)apps/notifications/src/modules/chain/services/feature-flags/feature-flags.ts(1 hunks)apps/notifications/src/modules/chain/services/rpc-url-resolver/rpc-url-resolver.service.ts(1 hunks)apps/provider-proxy/src/container.ts(3 hunks)apps/provider-proxy/src/services/feature-flags/feature-flags.service.ts(1 hunks)apps/provider-proxy/src/services/feature-flags/feature-flags.ts(1 hunks)apps/stats-web/src/config/env-config.schema.ts(1 hunks)apps/stats-web/src/services/api-url/api-url.service.ts(1 hunks)packages/database/chainDefinitions.ts(1 hunks)packages/net/scripts/generate.ts(0 hunks)packages/net/src/NetConfig/NetConfig.ts(2 hunks)packages/net/src/index.ts(1 hunks)
💤 Files with no reviewable changes (6)
- apps/api/env/.env.unit.test
- apps/api/env/.env.mainnet
- apps/api/env/.env.sample
- apps/api/src/billing/config/env.config.ts
- apps/api/env/.env.sandbox
- packages/net/scripts/generate.ts
🚧 Files skipped from review as they are similar to previous changes (40)
- apps/api/src/core/services/index.ts
- apps/provider-proxy/src/services/feature-flags/feature-flags.ts
- apps/api/test/functional/bids.spec.ts
- apps/notifications/src/modules/chain/services/feature-flags/feature-flags.ts
- apps/notifications/src/modules/chain/providers/stargate-client/stargate-client.provider.ts
- apps/notifications/src/modules/chain/chain.module.ts
- packages/database/chainDefinitions.ts
- apps/api/src/billing/services/dedupe-signing-client/dedupe-signing-client.service.ts
- apps/stats-web/src/config/env-config.schema.ts
- apps/api/test/functional/stale-anonymous-users-cleanup.spec.ts
- apps/api/test/functional/balances.spec.ts
- apps/notifications/src/modules/chain/services/feature-flags/feature-flags.service.ts
- apps/provider-proxy/src/container.ts
- apps/api/src/bid/controllers/bid/bid.controller.ts
- apps/api/src/core/services/http-service-wrapper/http-service-wrapper.ts
- apps/api/test/functional/create-deployment.spec.ts
- apps/api/src/dashboard/services/stats/stats.service.ts
- apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.spec.ts
- apps/notifications/src/modules/chain/config/env.config.ts
- apps/api/src/chain/services/block-http/block-http.service.ts
- apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts
- apps/api/src/billing/services/chain-error/chain-error.service.ts
- apps/api/test/functional/start-trial.spec.ts
- apps/notifications/src/modules/chain/providers/tendermint-client/tendermint-client.provider.ts
- apps/api/src/billing/services/balances/balances.service.ts
- apps/api/src/utils/env.ts
- apps/api/src/core/services/chain-config/chain-config.service.ts
- apps/provider-proxy/src/services/feature-flags/feature-flags.service.ts
- apps/api/src/proposal/services/proposal/proposal.service.ts
- apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts
- packages/net/src/NetConfig/NetConfig.ts
- apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts
- apps/api/test/functional/addresses.spec.ts
- apps/api/src/core/services/feature-flags/feature-flags.ts
- apps/notifications/src/modules/chain/services/rpc-url-resolver/rpc-url-resolver.service.ts
- apps/api/src/billing/services/financial-stats/financial-stats.service.ts
- apps/api/src/billing/services/chain-error/chain-error.service.spec.ts
- apps/api/test/functional/proposals.spec.ts
- apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts
- packages/net/src/index.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/address/services/address/address.service.tsapps/api/src/billing/providers/signing-client.provider.tsapps/api/src/utils/constants.tsapps/api/src/provider/services/provider/provider.service.tsapps/api/test/functional/validators.spec.tsapps/api/test/functional/lease-flow.spec.tsapps/api/src/validator/services/validator/validator.service.tsapps/api/src/chain/services/block-http/block-http.service.spec.tsapps/stats-web/src/services/api-url/api-url.service.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/api/test/functional/deployments.spec.tsapps/api/src/core/providers/http-sdk.provider.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/address/services/address/address.service.tsapps/api/src/billing/providers/signing-client.provider.tsapps/api/src/utils/constants.tsapps/api/src/provider/services/provider/provider.service.tsapps/api/test/functional/validators.spec.tsapps/api/test/functional/lease-flow.spec.tsapps/api/src/validator/services/validator/validator.service.tsapps/api/src/chain/services/block-http/block-http.service.spec.tsapps/stats-web/src/services/api-url/api-url.service.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/api/test/functional/deployments.spec.tsapps/api/src/core/providers/http-sdk.provider.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/test/functional/validators.spec.tsapps/api/test/functional/lease-flow.spec.tsapps/api/src/chain/services/block-http/block-http.service.spec.tsapps/api/test/functional/deployments.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-06-03T15:06:34.211Z
Learnt from: baktun14
PR: akash-network/console#1428
File: apps/api/src/deployment/controllers/deployment/deployment.controller.ts:0-0
Timestamp: 2025-06-03T15:06:34.211Z
Learning: The `getByOwnerAndDseq` method in `apps/api/src/deployment/controllers/deployment/deployment.controller.ts` is intentionally public without the `Protected` decorator because it serves public blockchain data from an indexer, following the pattern of public blockchain APIs.
Applied to files:
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts
🧬 Code Graph Analysis (5)
apps/api/src/utils/constants.ts (1)
apps/api/src/utils/env.ts (1)
env(3-27)
apps/api/test/functional/validators.spec.ts (1)
apps/api/src/utils/constants.ts (1)
apiProxyUrl(21-21)
apps/api/src/validator/services/validator/validator.service.ts (3)
apps/api/src/proposal/services/proposal/proposal.service.ts (1)
singleton(7-75)apps/api/src/validator/repositories/validator/validator.repository.ts (1)
singleton(5-26)apps/api/src/validator/controllers/validator/validator.controller.ts (1)
singleton(6-17)
apps/stats-web/src/services/api-url/api-url.service.ts (3)
apps/stats-web/src/lib/apiUtils.ts (1)
ApiUrlService(6-57)apps/stats-web/src/config/env-config.schema.ts (2)
ServerEnvConfig(30-30)BrowserEnvConfig(29-29)packages/network-store/src/network.config.ts (1)
MAINNET_ID(5-5)
apps/api/test/functional/deployments.spec.ts (2)
apps/api/src/utils/constants.ts (3)
apiProxyUrl(21-21)betaTypeVersion(17-17)betaTypeVersionMarket(18-18)apps/api/test/seeders/lease-api-response.seeder.ts (1)
LeaseApiResponseSeeder(61-125)
🪛 dotenv-linter (3.3.0)
apps/api/env/.env.functional.test
[warning] 43-43: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
⏰ 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). (4)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (23)
apps/api/src/billing/providers/signing-client.provider.ts (3)
9-9: ChainConfigService import — looks correctImport path and usage align with the new DI dependency.
26-34: UAKT_TOP_UP client: ChainConfigService injection — looks goodPlacement mirrors the MANAGED client and aligns with the new constructor.
39-47: USDC_TOP_UP client: ChainConfigService injection — looks goodConsistent with the other registrations; no issues spotted.
apps/api/src/chain/services/block-http/block-http.service.spec.ts (1)
1-2: Good use of jest-mock-extended with type-only importsTyped mocks via MockProxy and mock() align well with our test guidelines.
apps/stats-web/src/services/api-url/api-url.service.ts (1)
2-2: LGTM: centralized network constants importUsing MAINNET_ID/SANDBOX_ID/TESTNET_ID from @akashnetwork/network-store keeps identifiers consistent across apps.
apps/api/src/address/services/address/address.service.ts (2)
7-7: Switch to CosmosHttpServiceWrapper aligns with proxy architecture — LGTMImporting the wrapper is consistent with the PR’s objective to centralize base URL/proxy resolution.
17-17: CosmosHttpServiceWrapper is properly registered for DI
TheCosmosHttpServiceWrapperclass inapps/api/src/core/services/http-service-wrapper/http-service-wrapper.tsis annotated with@singleton(), so tsyringe will resolve it at runtime. No further registration steps are required.apps/api/src/validator/services/validator/validator.service.ts (1)
4-4: Migration to CosmosHttpServiceWrapper is correct and consistent.Constructor injection and all read paths (list/getByAddress + subsequent list) are properly routed through the wrapper, matching the PR direction.
Also applies to: 11-11, 16-16, 43-43, 52-52
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (5)
3-3: Type-only import is a good callUsing
import typekeeps the runtime bundle clean and avoids accidental circular deps. Looks good.
10-10: Good decoupling via HTTP service wrappersImporting the wrappers centralizes base URL/proxy decisions and makes this service easier to test and evolve.
22-24: Constructor DI swap to wrappers looks correctThe switch to wrapper dependencies is consistent. Assuming those classes are decorated with
@singleton()(or registered), tsyringe will resolve them fine.
40-41: Leases call via wrapper looks goodRouting through
leaseHttpServiceWrapper.list({ owner, dseq })keeps concerns separated and consistent with the proxy-aware setup.
125-131: LGTM: listWithResources now consistently uses wrappersPassing
reverseandcountTotalis clear, and fetching all active leases once per owner helps avoid N+1 calls per deployment.apps/api/env/.env.functional.test (1)
5-5: Do not remove legacy env vars yet – they’re still in use
We ran a global search and foundRPC_NODE_ENDPOINTandAPI_NODE_ENDPOINTreferenced across multiple modules and test suites. Removing them now would break functionality. For example:
- apps/notifications (test seeders & functional specs)
- apps/notifications/src/modules/alert/providers/http-sdk.provider.ts
- apps/notifications/src/modules/chain/services/rpc-url-resolver
- apps/api/test/services/test-wallet.service.ts
- apps/api/src/deployment and billing services (gpu-bids-creator, financial-stats)
- dozens of apps/api/test/functional/*.spec.ts using
nock(module.get(ConfigService).getOrThrow("API_NODE_ENDPOINT"))- apps/api/src/utils/constants.ts exports based on the legacy vars
Suggested migration path:
- Continue to support
RPC_NODE_ENDPOINT/API_NODE_ENDPOINTuntil all code and tests are updated to usePROXY_RPC_URL/PROXY_API_URL.- Incrementally replace
configService.getOrThrow("…_NODE_ENDPOINT")and seeders/constants to point at the new proxy URLs.- Once no legacy references remain, safely remove the old vars from
.env.functional.test, CI configs, and documentation.Likely an incorrect or invalid review comment.
apps/api/src/provider/services/provider/provider.service.ts (2)
10-10: LGTM: Switched to ProviderHttpServiceWrapper and injected via DIThis aligns the service with the new proxy-aware HTTP layer. Ensure the wrapper class is registered in the DI container in all test/runtime modules.
Also applies to: 29-29
40-41: LGTM: Provider lookup routed through wrapperUsing wrapper.getProvider(...) centralizes base URL resolution and proxy logic. Double-check wrapper response typing guarantees providerResponse.provider.host_uri exists across networks.
Also applies to: 80-81
apps/api/test/functional/validators.spec.ts (1)
5-5: Remove lingering direct node endpoint references in testsThe grep scan still shows direct references to
API_NODE_ENDPOINTandRPC_NODE_ENDPOINTin several test files. All tests must use the new proxy base URL (apiProxyUrlorPROXY_RPC_URLfrom@src/utils/constants) instead of the old config keys. Please update the following:
- apps/notifications/test/functional/balance-alert.spec.ts (line 141)
Replacemodule.get(ConfigService).getOrThrow("API_NODE_ENDPOINT")withapiProxyUrl.- apps/notifications/test/functional/deployment-alert-crud.spec.ts (line 146)
Replacemodule.get(ConfigService).getOrThrow("API_NODE_ENDPOINT")withapiProxyUrl.- apps/notifications/test/seeders/alert-config.seeder.ts (line 9)
Replace seededAPI_NODE_ENDPOINTwith the proxy URL field (e.g.PROXY_RPC_URL).- apps/api/test/services/test-wallet.service.ts (lines 23, 88)
- Change
baseURL: config!.API_NODE_ENDPOINT→baseURL: apiProxyUrl- Change
SigningStargateClient.connectWithSigner(config!.RPC_NODE_ENDPOINT, …)→connectWithSigner(apiProxyUrl, …)Ensure you import the proxy URL constant and remove all legacy endpoint keys from these tests.
⛔ Skipped due to learnings
Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/query-by-in-tests.mdc:0-0 Timestamp: 2025-07-21T08:24:27.953Z Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` filesapps/api/test/functional/lease-flow.spec.ts (1)
11-11: LGTM: Resolved BlockHttpServiceWrapper via DIUsing the wrapper keeps the test aligned with proxy-aware infrastructure.
Confirm the DI container registers BlockHttpServiceWrapper in the test module bootstrap to avoid runtime resolution errors.
Also applies to: 26-26
apps/api/src/utils/constants.ts (1)
21-22: LGTM: Introduced proxy URL constantsapiProxyUrl and rpcProxyUrl sourced from env are straightforward and make tests and wrappers consistent.
apps/api/src/core/providers/http-sdk.provider.ts (1)
1-1: Scoped DI registrations — good reduction of surface areaMoving to explicit registrations for only GitHub, CoinGecko, and Node services improves clarity and avoids wiring unused services. Good alignment with the proxy-aware direction.
apps/api/test/functional/deployments.spec.ts (3)
14-14: Wrapper adoption for BlockHttpService — LGTMResolving and stubbing
BlockHttpServiceWrapper.getCurrentHeightaligns with the new proxy-aware service layer. Looks good.Also applies to: 36-36, 67-67
192-201: Per-dseq mocks via proxy URL — LGTMSwitching to
apiProxyUrland wiring per-dseq nocks is clear and in line with the proxy approach.
20-20: Verified: PROXY_API_URL includes URL scheme
ThePROXY_API_URLinapps/api/env/.env.functional.testis set tohttps://api.sandbox-01.aksh.pw:443, andapiProxyUrlsimply re-exports that value. Nock will receive an absolute URL, so no changes are needed.
apps/api/src/chain/services/block-http/block-http.service.spec.ts
Outdated
Show resolved
Hide resolved
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
apps/api/src/chain/services/block-http/block-http.service.spec.ts (1)
20-25: Optional: destructure setup param with a default value to simplify and match house styleGood job meeting the test guidelines: single inline-typed param, no explicit return type, and setup at the bottom of the root describe. As a small readability improvement, destructuring with a default
{}removes the need for optional chaining and aligns with our common setup pattern.Apply this diff:
- function setup(input?: { blockHttpServiceWrapper?: MockProxy<BlockHttpServiceWrapper> }) { - const blockHttpServiceWrapper = input?.blockHttpServiceWrapper ?? mock<BlockHttpServiceWrapper>(); - const service = new BlockHttpServiceClass(blockHttpServiceWrapper); - - return { blockHttpServiceWrapper, service }; - } + function setup({ + blockHttpServiceWrapper = mock<BlockHttpServiceWrapper>(), + }: { blockHttpServiceWrapper?: MockProxy<BlockHttpServiceWrapper> } = {}) { + const service = new BlockHttpServiceClass(blockHttpServiceWrapper); + return { blockHttpServiceWrapper, service }; + }apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (2)
32-37: Resolved: safe optional chaining before.includes(fixes prior runtime TypeError)The added
?.before.includesprevents calling.includeson undefined and addresses the earlier bot comment.
185-193: Possible runtime TypeError: optional chaining missing before.includes
deploymentData.message?.toLowerCase()can be undefined whenmessageis falsy; calling.includeson that will throw. Mirror the earlier fix by chaining?.before.includes.Apply this diff:
- if (deploymentData && typeof deploymentData === "object" && "code" in deploymentData) { - if (deploymentData.message?.toLowerCase().includes("deployment not found")) { + if (deploymentData && typeof deploymentData === "object" && "code" in deploymentData) { + if (deploymentData.message?.toLowerCase()?.includes("deployment not found")) { return null; } else { throw new Error(deploymentData.message); } }apps/stats-web/src/services/api-url/api-url.service.ts (1)
20-50: Don’t return undefined when proxy URLs are missing; add explicit return type and safe fallbackCurrent logic returns PROXY_API_URL / NEXT_PUBLIC_PROXY_API_URL unconditionally when the flag is enabled on MAINNET. If those envs are empty/missing, callers will receive an invalid base URL. Also, the method lacks an explicit return type.
This mirrors a previously raised concern; reapplying the fix here prevents runtime failures.
Apply this diff:
- getBaseApiUrlFor(network: NetworkId) { + getBaseApiUrlFor(network: NetworkId): string { if ("BASE_API_MAINNET_URL" in this.config) { - if (this.config.USE_PROXY_URLS && network === MAINNET_ID) { - return this.config.PROXY_API_URL; - } + if (this.config.USE_PROXY_URLS && network === MAINNET_ID) { + const proxyUrl = this.config.PROXY_API_URL; + if (proxyUrl && proxyUrl.trim()) return proxyUrl; + } switch (network) { case TESTNET_ID: return this.config.BASE_API_TESTNET_URL; case SANDBOX_ID: return this.config.BASE_API_SANDBOX_URL; default: return this.config.BASE_API_MAINNET_URL; } } if ("NEXT_PUBLIC_BASE_API_MAINNET_URL" in this.config) { - if (this.config.NEXT_PUBLIC_USE_PROXY_URLS && network === MAINNET_ID) { - return this.config.NEXT_PUBLIC_PROXY_API_URL; - } + if (this.config.NEXT_PUBLIC_USE_PROXY_URLS && network === MAINNET_ID) { + const proxyUrl = this.config.NEXT_PUBLIC_PROXY_API_URL; + if (proxyUrl && proxyUrl.trim()) return proxyUrl; + } switch (network) { case TESTNET_ID: return this.config.NEXT_PUBLIC_BASE_API_TESTNET_URL; case SANDBOX_ID: return this.config.NEXT_PUBLIC_BASE_API_SANDBOX_URL; default: return this.config.NEXT_PUBLIC_BASE_API_MAINNET_URL; } } + throw new Error("ApiUrlService: Missing base API URL configuration"); }Optional follow-ups:
- Add unit tests covering:
- Proxy flag true + proxy URL set → proxy returned for MAINNET.
- Proxy flag true + proxy URL missing/empty → falls back to base MAINNET URL.
- TESTNET/SANDBOX unaffected regardless of flag.
Do you want me to scaffold these tests?
🧹 Nitpick comments (8)
apps/api/src/chain/services/block-http/block-http.service.spec.ts (1)
7-17: Tighten assertions: verify call count (and optionally args) for stronger contractMinor: you can strengthen the test by asserting the call count (and optionally the call args) to avoid accidental multiple invocations or signature drift.
Apply this diff to refine the expectations:
- expect(blockHttpServiceWrapper.getCurrentHeight).toHaveBeenCalled(); + expect(blockHttpServiceWrapper.getCurrentHeight).toHaveBeenCalledTimes(1); + expect(blockHttpServiceWrapper.getCurrentHeight).toHaveBeenCalledWith();apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (2)
87-90: Typo: renamedeploymentReponsetodeploymentResponseMinor readability nit to avoid confusion.
Apply this diff:
- const deploymentReponse = await this.deploymentHttpServiceWrapper.loadDeploymentList(owner, "active", pagination); - const deployments = deploymentReponse.deployments; - const total = parseInt(deploymentReponse.pagination.total, 10); + const deploymentResponse = await this.deploymentHttpServiceWrapper.loadDeploymentList(owner, "active", pagination); + const deployments = deploymentResponse.deployments; + const total = parseInt(deploymentResponse.pagination.total, 10);
91-94: Capture PromisePool errors for observabilityYou’re ignoring
PromisePoolerrors. Consider destructuringerrorsand logging/recording them to avoid silent failures while still returning partial results.apps/notifications/src/modules/chain/providers/stargate-client/stargate-client.provider.spec.ts (2)
17-18: Add an expectation that the resolver is invoked exactly once.This asserts the intended interaction and guards against accidental multiple resolutions later.
Apply this addition near the existing expectations:
expect(MockStargateClient.connect).toHaveBeenCalledWith(rpcNodeEndpoint); + expect(rpcUrlResolver.getRpcUrl).toHaveBeenCalledTimes(1); expect(client).toBe(mockClient);Optional: to reduce non-determinism and a dependency on faker at test time, consider using a literal like "https://rpc.example.org" instead of faker.internet.url(). This is stylistic; functionally fine either way.
19-19: Minimize unsafe casting; tighten static shape at declaration.The double cast to typeof StargateClient is common in tests but can be localized to where the mock is declared, and we can partially type-check the static connect shape. This keeps the call site cleaner and provides better compile-time checks without using any.
Suggested change (move the cast to the declaration and assert the method shape with satisfies):
// Replace the current declaration at Lines 12-14 with: const MockStargateClient = ({ connect: jest.fn().mockResolvedValue(mockClient) } as unknown) as typeof StargateClient; // Alternatively, get some compile-time checking of the shape of connect: const MockStargateClientShape = { connect: jest.fn().mockResolvedValue(mockClient) } satisfies { connect: (endpoint: string) => Promise<StargateClient> }; // Then cast once where needed: const MockStargateClient = MockStargateClientShape as unknown as typeof StargateClient; // With either of the above, this line becomes: const client = await createStargateClientFactory(MockStargateClient)(rpcUrlResolver);If you’d like to avoid the cast entirely, we can adjust the provider factory to depend on a minimal static interface (e.g., { connect: (...) => Promise }) rather than the full typeof StargateClient. That’s a broader design change but makes testing simpler.
apps/provider-proxy/src/container.ts (1)
18-19: Wire flag value via a local const for readability and easier instrumentation.Minor readability improvement that also makes it trivial to log or trace which mode is active without re-calling the flag service.
Apply this diff:
- const featureFlagsService = new FeatureFlagsService(); - const netConfig = createNetConfig({ useProxyUrls: featureFlagsService.isEnabled(FeatureFlags.USE_PROXY_URLS) }); + const featureFlagsService = new FeatureFlagsService(); + const useProxyUrls = featureFlagsService.isEnabled(FeatureFlags.USE_PROXY_URLS); + const netConfig = createNetConfig({ useProxyUrls });apps/stats-web/src/config/browser-env.config.ts (1)
10-12: Add NEXT_PUBLIC_PROXY_API_URL mapping to browser-env configThe
browserEnvSchemaalready definesNEXT_PUBLIC_PROXY_API_URL(with a default), butapps/stats-web/src/config/browser-env.config.tsisn’t mapping it fromprocess.env. This means any custom proxy URL set via env vars will be ignored. Please add:NEXT_PUBLIC_LOG_LEVEL: process.env.NEXT_PUBLIC_LOG_LEVEL, NEXT_PUBLIC_USE_PROXY_URLS: process.env.NEXT_PUBLIC_USE_PROXY_URLS, + NEXT_PUBLIC_PROXY_API_URL: process.env.NEXT_PUBLIC_PROXY_API_URL, });apps/stats-web/src/services/api-url/api-url.service.ts (1)
9-17: Constructor typing looks good; align runtime config to actually include proxy keysThe union types correctly include PROXY_API_URL/USE_PROXY_URLS (server) and NEXT_PUBLIC_PROXY_API_URL/NEXT_PUBLIC_USE_PROXY_URLS (browser). Ensure the passed browser config (browser-env.config.ts) exports NEXT_PUBLIC_PROXY_API_URL; otherwise, accessing it will yield undefined at runtime.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
apps/api/src/chain/services/block-http/block-http.service.spec.ts(1 hunks)apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts(7 hunks)apps/api/src/validator/services/validator/validator.service.ts(3 hunks)apps/notifications/src/modules/chain/providers/stargate-client/stargate-client.provider.spec.ts(1 hunks)apps/provider-proxy/src/container.ts(3 hunks)apps/stats-web/src/config/browser-env.config.ts(1 hunks)apps/stats-web/src/config/env-config.schema.ts(1 hunks)apps/stats-web/src/services/api-url/api-url.service.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api/src/validator/services/validator/validator.service.ts
- apps/stats-web/src/config/env-config.schema.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/chain/services/block-http/block-http.service.spec.tsapps/notifications/src/modules/chain/providers/stargate-client/stargate-client.provider.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/chain/services/block-http/block-http.service.spec.tsapps/provider-proxy/src/container.tsapps/stats-web/src/config/browser-env.config.tsapps/notifications/src/modules/chain/providers/stargate-client/stargate-client.provider.spec.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/stats-web/src/services/api-url/api-url.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/chain/services/block-http/block-http.service.spec.tsapps/provider-proxy/src/container.tsapps/stats-web/src/config/browser-env.config.tsapps/notifications/src/modules/chain/providers/stargate-client/stargate-client.provider.spec.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/stats-web/src/services/api-url/api-url.service.ts
🧠 Learnings (9)
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Applied to files:
apps/api/src/chain/services/block-http/block-http.service.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Applied to files:
apps/api/src/chain/services/block-http/block-http.service.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Applied to files:
apps/api/src/chain/services/block-http/block-http.service.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Applied to files:
apps/api/src/chain/services/block-http/block-http.service.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Applied to files:
apps/api/src/chain/services/block-http/block-http.service.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block in test files
Applied to files:
apps/api/src/chain/services/block-http/block-http.service.spec.ts
📚 Learning: 2025-07-21T08:24:24.269Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies 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.ts
📚 Learning: 2025-08-12T13:52:38.708Z
Learnt from: stalniy
PR: akash-network/console#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/stats-web/src/config/browser-env.config.ts
📚 Learning: 2025-06-03T15:06:34.211Z
Learnt from: baktun14
PR: akash-network/console#1428
File: apps/api/src/deployment/controllers/deployment/deployment.controller.ts:0-0
Timestamp: 2025-06-03T15:06:34.211Z
Learning: The `getByOwnerAndDseq` method in `apps/api/src/deployment/controllers/deployment/deployment.controller.ts` is intentionally public without the `Protected` decorator because it serves public blockchain data from an indexer, following the pattern of public blockchain APIs.
Applied to files:
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts
🧬 Code Graph Analysis (3)
apps/provider-proxy/src/container.ts (4)
apps/provider-proxy/src/services/feature-flags/feature-flags.service.ts (1)
FeatureFlagsService(3-18)packages/net/src/index.ts (3)
netConfig(3-3)createNetConfig(6-8)SupportedChainNetworks(4-4)apps/provider-proxy/src/services/feature-flags/feature-flags.ts (1)
FeatureFlags(1-3)packages/net/src/NetConfig/NetConfig.ts (1)
SupportedChainNetworks(3-3)
apps/notifications/src/modules/chain/providers/stargate-client/stargate-client.provider.spec.ts (1)
apps/notifications/src/modules/chain/providers/stargate-client/stargate-client.provider.ts (1)
createStargateClientFactory(6-10)
apps/stats-web/src/services/api-url/api-url.service.ts (3)
apps/stats-web/src/lib/apiUtils.ts (1)
ApiUrlService(6-57)apps/stats-web/src/config/env-config.schema.ts (2)
ServerEnvConfig(32-32)BrowserEnvConfig(31-31)packages/network-store/src/network.config.ts (1)
MAINNET_ID(5-5)
⏰ 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). (10)
- GitHub Check: test-build
- 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: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (11)
apps/api/src/chain/services/block-http/block-http.service.spec.ts (1)
1-5: LGTM: proper use of jest-mock-extended and type-only importsImports are lean and type-safe; using jest-mock-extended is aligned with our test guidelines. No
jest.mock()usage, which is good.apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (4)
40-40: LGTM: switched to wrapper for leases lookupThe call to
leaseHttpServiceWrapper.list({ owner, dseq })is consistent with the new abstraction.
125-131: LGTM: wrapper-based pagination and active-lease queryCentralizing via wrappers is consistent with the proxy-aware config direction.
202-206: LGTM: null guard + wrapper-based lease queryThe early null return prevents unnecessary downstream calls; the lease lookup via wrapper is consistent.
3-3: DI wiring verified: wrappers properly exported and decoratedAll
HttpServiceWrapperclasses—includingDeploymentHttpServiceWrapperandLeaseHttpServiceWrapper—are annotated with@singleton()and exported inapps/api/src/core/services/http-service-wrapper/http-service-wrapper.ts. The entire module is re-exported inapps/api/src/core/services/index.ts, ensuring tsyringe can resolve constructor injection at runtime.apps/notifications/src/modules/chain/providers/stargate-client/stargate-client.provider.spec.ts (1)
5-5: Good switch to RpcUrlResolverService (aligns with new proxy-aware design).Using RpcUrlResolverService here keeps the provider test consistent with the new resolution flow and removes direct ConfigService coupling. Looks good.
apps/provider-proxy/src/container.ts (3)
7-8: Feature flag wiring: imports look correct and consistent.Imports align with the FeatureFlagsService contract (literal "use_proxy_urls"), ensuring type correctness and preventing drift.
26-26: Comment-only change acknowledged.No action needed.
51-51: Exposing featureFlagsService from the container is helpful.Returning featureFlagsService improves observability/testing and keeps configuration discoverable at the composition root.
apps/stats-web/src/config/browser-env.config.ts (1)
10-11: Boolean coercion is correctly applied for NEXT_PUBLIC_USE_PROXY_URLSThe Zod schema uses
coercedBoolean().optional().default("false"), so string values like"false"are coerced to the booleanfalse. No further changes required.apps/stats-web/src/services/api-url/api-url.service.ts (1)
2-2: Importing MAINNET_ID is appropriate for explicit mainnet gatingPulling MAINNET_ID from network-store to gate proxy usage is correct and improves readability.
apps/notifications/src/modules/chain/providers/stargate-client/stargate-client.provider.spec.ts
Show resolved
Hide resolved
apps/api/src/core/services/http-service-wrapper/http-service-wrapper.ts
Outdated
Show resolved
Hide resolved
apps/provider-proxy/src/services/feature-flags/feature-flags.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/billing/services/financial-stats/financial-stats.service.ts (1)
1-21: Convert type-only DI imports to value importsThe
import type { CosmosHttpService }on line 2 is erased at compile time, causing tsyringe to be unable to resolve and inject the service at runtime. Please:• In apps/api/src/billing/services/financial-stats/financial-stats.service.ts (lines 1–3 and similarly at 56–60), change:
- import type { CosmosDistributionCommunityPoolResponse, CosmosHttpService } from "@akashnetwork/http-sdk"; + import { CosmosDistributionCommunityPoolResponse, CosmosHttpService } from "@akashnetwork/http-sdk";• Audit the codebase for any other DI tokens (services, repositories, handlers, etc.) brought in via
import typeand convert them to regular (value) imports. For example:rg -nP --type=ts 'import\s+type\s+\{\s*\w+(Service|Repository|Handler)\b' -C1Ensuring all injectable classes are imported as values will prevent runtime injection failures.
🧹 Nitpick comments (12)
apps/provider-proxy/src/services/feature-flags/feature-flags.service.ts (4)
4-5: Remove the no-op constructorThe empty constructor is unnecessary and can be removed for clarity.
export class FeatureFlagsService { - constructor() {} - isEnabled(featureFlag: FeatureFlagValue): boolean {
1-1: Prefer using the FeatureFlags constant and add exhaustive checkingUsing the constant avoids string typos, and exhaustive checking will surface unhandled flags at compile-time when the union grows.
-import type { FeatureFlagValue } from "./feature-flags"; +import { FeatureFlags, type FeatureFlagValue } from "./feature-flags"; @@ isEnabled(featureFlag: FeatureFlagValue): boolean { switch (featureFlag) { - case "use_proxy_urls": + case FeatureFlags.USE_PROXY_URLS: return process.env.USE_PROXY_URLS === "true"; - default: - return false; + default: { + const _exhaustiveCheck: never = featureFlag; + throw new Error(`Unhandled feature flag: ${_exhaustiveCheck}`); + } } }Also applies to: 8-12
9-9: Make env parsing tolerant (case-insensitive, 1/yes)Slightly more robust truthy parsing helps avoid env drift across environments.
- return process.env.USE_PROXY_URLS === "true"; + const v = String(process.env.USE_PROXY_URLS ?? "").toLowerCase(); + return v === "true" || v === "1" || v === "yes" || v === "y";
3-14: Add minimal unit tests for env-driven behaviorRecommend tests covering:
- default (unset) → false
- USE_PROXY_URLS="true" → true
- common truthy variants ("1","YES") if you adopt tolerant parsing.
I can draft a small test suite (Jest or your current runner) that mocks process.env per test case. Want me to add it?
apps/api/src/billing/controllers/wallet/wallet.controller.spec.ts (2)
172-191: Optional: Isolate DI per test using a child container instead of the global rootContainerUsing the shared rootContainer risks cross-test leakage. Consider creating a child container inside setup() and registering mocks on it, then returning the child container. This improves test isolation without changing call sites.
Example pattern:
- Create a child container and register everything on it.
- Return the child container from setup(), not the root.
Additional code (outside the selected range) to illustrate:
function setup(input?: { /* ... */ }) { const container = rootContainer.createChildContainer(); container.register(AuthService, { useValue: /* ... */ }); // register all other dependencies on `container` instead of `rootContainer` return container; }
142-193: Optional: Make setup() return the object under test (WalletController) to match test guidelinesOur spec guidelines prefer setup() to construct and return the SUT. Returning the WalletController (and any mocks you want to assert on) avoids leaking DI concerns into test bodies.
Proposed refactor (outside selected range, minimal call-site churn):
- function setup(input?: { user?: UserOutput; enabledFeatures?: FeatureFlagValue[]; hasPaymentMethods?: boolean; hasDuplicateTrialAccount?: boolean }) { + function setup(input?: { user?: UserOutput; enabledFeatures?: FeatureFlagValue[]; hasPaymentMethods?: boolean; hasDuplicateTrialAccount?: boolean }) { // (Optionally create a child container here as suggested above) - rootContainer.register(AuthService, { /* ... */ }); + rootContainer.register(AuthService, { /* ... */ }); // ... all registrations ... - return rootContainer; + const walletController = rootContainer.resolve(WalletController); + return { walletController, container: rootContainer }; }Then in tests replace:
- const walletController = container.resolve(WalletController);
with:- const { walletController } = container;
apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts (1)
85-93: Use ChainNetworkConfigService for RPC URL — good; consider adding a trace for easier triageSwitching to chain config is the right move. As a small improvement, emit a debug log with the resolved RPC URL and tighten the error message to ease production triage.
Suggested diff:
private async initClient() { - const rpcUrl = this.chainNetworkConfigService.getBaseRpcUrl(); - if (!rpcUrl) throw new Error("RPC url for the batch signing client is not set."); + const rpcUrl = this.chainNetworkConfigService.getBaseRpcUrl(); + if (!rpcUrl) throw new Error("[BatchSigningClientService] RPC URL is not set."); + this.logger.debug({ event: "BATCH_SIGNING_CLIENT_RESOLVED_RPC_URL", rpcUrl }); return await backOff( () => - this.connectWithSigner(rpcUrl, this.wallet, { + this.connectWithSigner(rpcUrl, this.wallet, { registry: this.registry }).then(async client => { this.chainId = await client.getChainId(); return client; }),apps/api/src/core/services/chain-network-config/chain-network-config.service.ts (2)
19-25: Validate env.NETWORK and cache it to avoid silent misconfigurationIf env.NETWORK is misspelled or unsupported, downstream calls may behave unexpectedly. Validate once and cache the network to tighten feedback loops.
Proposed diff:
export class ChainNetworkConfigService { private readonly netConfig: ReturnType<typeof createNetConfig>; + private readonly network: SupportedChainNetworks; constructor(private readonly featureFlagsService: FeatureFlagsService) { this.netConfig = createNetConfig({ useProxyUrls: this.featureFlagsService.isEnabled("use_proxy_urls"), proxyApiUrl: env.PROXY_API_URL, proxyRpcUrl: env.PROXY_RPC_URL }); + + const candidate = env.NETWORK as SupportedChainNetworks; + const supported = this.netConfig.getSupportedNetworks(); + if (!supported.includes(candidate)) { + throw new Error(`Invalid NETWORK '${env.NETWORK}'. Supported values: ${supported.join(", ")}`); + } + this.network = candidate; } getBaseAPIUrl(): string { - return this.netConfig.getBaseAPIUrl(env.NETWORK as SupportedChainNetworks); + return this.netConfig.getBaseAPIUrl(this.network); } getBaseRpcUrl(): string { - return this.netConfig.getBaseRpcUrl(env.NETWORK as SupportedChainNetworks); + return this.netConfig.getBaseRpcUrl(this.network); }
31-34: Optional: Unit tests for feature-flagged proxy behaviorAdd a small spec to assert that:
- With use_proxy_urls disabled, base URLs match direct network endpoints.
- With use_proxy_urls enabled and proxy envs set, base URLs switch to proxies.
I can scaffold a spec using jest-mock-extended for FeatureFlagsService; let me know if you want me to push it.
apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.spec.ts (1)
177-233: Align setup() with project testing conventions (accept a single, typed param)Per the repo’s testing guidelines, setup should accept a single parameter with an inline type definition. Make rpcUrl configurable while keeping current defaults; no call sites need changing.
Apply this diff:
- function setup() { + function setup({ rpcUrl = "http://localhost:26657" }: { rpcUrl?: string }) { @@ - const mockChainConfigService = mock<ChainNetworkConfigService>(); - mockChainConfigService.getBaseRpcUrl.mockReturnValue("http://localhost:26657"); + const mockChainConfigService = mock<ChainNetworkConfigService>(); + mockChainConfigService.getBaseRpcUrl.mockReturnValue(rpcUrl); @@ - const service = new BatchSigningClientService(mockConfig, mockWallet, mockRegistry, connectWithSigner, mockChainConfigService); + const service = new BatchSigningClientService(mockConfig, mockWallet, mockRegistry, connectWithSigner, mockChainConfigService);apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts (1)
182-186: Prefer BlockHttpService over raw axios for height (retries, consistency)We now provide BlockHttpService with baseURL from ChainNetworkConfigService (and http-sdk has retry/backoff patterns elsewhere). Using it keeps concerns centralized and reduces bespoke error handling.
Apply this refactor:
- private async getCurrentHeight() { - const apiNodeUrl = this.chainConfigService.getBaseAPIUrl(); - const response = await axios.get(`${apiNodeUrl}/blocks/latest`); - - const height = parseInt(response.data.block.header.height); - - if (isNaN(height)) throw new Error("Failed to get current height"); - - return height; - } + private async getCurrentHeight() { + // Reuse http-sdk’s REST wrapper + const response = await axios.get(`${this.chainConfigService.getBaseAPIUrl()}/blocks/latest`); + const height = parseInt(response.data.block.header.height); + if (isNaN(height)) throw new Error("Failed to get current height"); + return height; + }Or, if you prefer DI (recommended), inject BlockHttpService and call blockHttpService.getCurrentHeight().
apps/api/src/billing/services/financial-stats/financial-stats.service.ts (1)
56-60: Reuse CosmosHttpService for community pool (drop raw axios and extra dependency)We already inject CosmosHttpService (with retry/backoff in http-sdk). Use it instead of manual axios + baseURL resolution; also makes ChainNetworkConfigService in this class unnecessary.
Apply this diff:
- async getCommunityPoolUsdc() { - const apiNodeUrl = this.chainNetworkConfigService.getBaseAPIUrl(); - const communityPoolData = await axios.get<CosmosDistributionCommunityPoolResponse>(`${apiNodeUrl}/cosmos/distribution/v1beta1/community_pool`); - return parseFloat(communityPoolData.data.pool.find(x => x.denom === USDC_IBC_DENOMS.mainnetId)?.amount || "0"); - } + async getCommunityPoolUsdc() { + const pool = await this.cosmosHttpService.getCommunityPool(); + return parseFloat(pool.find(x => x.denom === USDC_IBC_DENOMS.mainnetId)?.amount || "0"); + }And remove now-unused imports/deps:
-import axios from "axios"; -import { ChainNetworkConfigService } from "@src/core/services/chain-network-config/chain-network-config.service"; @@ - private readonly cosmosHttpService: CosmosHttpService, - private readonly chainNetworkConfigService: ChainNetworkConfigService + private readonly cosmosHttpService: CosmosHttpService
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
apps/api/src/address/services/address/address.service.ts(1 hunks)apps/api/src/billing/controllers/wallet/wallet.controller.spec.ts(2 hunks)apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.spec.ts(3 hunks)apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts(3 hunks)apps/api/src/billing/providers/signing-client.provider.ts(4 hunks)apps/api/src/billing/services/chain-error/chain-error.service.spec.ts(2 hunks)apps/api/src/billing/services/dedupe-signing-client/dedupe-signing-client.service.ts(3 hunks)apps/api/src/billing/services/financial-stats/financial-stats.service.ts(3 hunks)apps/api/src/chain/services/block-http/block-http.service.spec.ts(1 hunks)apps/api/src/chain/services/block-http/block-http.service.ts(1 hunks)apps/api/src/core/providers/http-sdk.provider.ts(1 hunks)apps/api/src/core/services/chain-network-config/chain-network-config.service.ts(1 hunks)apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts(3 hunks)apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts(4 hunks)apps/api/test/functional/deployments.spec.ts(7 hunks)apps/provider-proxy/src/services/feature-flags/feature-flags.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/api/src/billing/services/dedupe-signing-client/dedupe-signing-client.service.ts
- apps/api/src/address/services/address/address.service.ts
- apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts
- apps/api/src/billing/providers/signing-client.provider.ts
- apps/api/test/functional/deployments.spec.ts
- apps/api/src/billing/services/chain-error/chain-error.service.spec.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/billing/controllers/wallet/wallet.controller.spec.tsapps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.spec.tsapps/api/src/chain/services/block-http/block-http.service.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/billing/controllers/wallet/wallet.controller.spec.tsapps/provider-proxy/src/services/feature-flags/feature-flags.service.tsapps/api/src/core/services/chain-network-config/chain-network-config.service.tsapps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.tsapps/api/src/chain/services/block-http/block-http.service.tsapps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.spec.tsapps/api/src/chain/services/block-http/block-http.service.spec.tsapps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.tsapps/api/src/billing/services/financial-stats/financial-stats.service.tsapps/api/src/core/providers/http-sdk.provider.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/billing/controllers/wallet/wallet.controller.spec.tsapps/provider-proxy/src/services/feature-flags/feature-flags.service.tsapps/api/src/core/services/chain-network-config/chain-network-config.service.tsapps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.tsapps/api/src/chain/services/block-http/block-http.service.tsapps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.spec.tsapps/api/src/chain/services/block-http/block-http.service.spec.tsapps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.tsapps/api/src/billing/services/financial-stats/financial-stats.service.tsapps/api/src/core/providers/http-sdk.provider.ts
🧠 Learnings (8)
📚 Learning: 2025-07-21T08:24:24.269Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies 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.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Applied to files:
apps/api/src/chain/services/block-http/block-http.service.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Applied to files:
apps/api/src/chain/services/block-http/block-http.service.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Applied to files:
apps/api/src/chain/services/block-http/block-http.service.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Applied to files:
apps/api/src/chain/services/block-http/block-http.service.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Applied to files:
apps/api/src/chain/services/block-http/block-http.service.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block in test files
Applied to files:
apps/api/src/chain/services/block-http/block-http.service.spec.ts
📚 Learning: 2025-05-28T20:42:58.200Z
Learnt from: jzsfkzm
PR: akash-network/console#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
🧬 Code Graph Analysis (8)
apps/provider-proxy/src/services/feature-flags/feature-flags.service.ts (3)
apps/api/src/core/services/feature-flags/feature-flags.ts (1)
FeatureFlagValue(8-8)apps/notifications/src/modules/chain/services/feature-flags/feature-flags.ts (1)
FeatureFlagValue(5-5)apps/provider-proxy/src/services/feature-flags/feature-flags.ts (1)
FeatureFlagValue(5-5)
apps/api/src/core/services/chain-network-config/chain-network-config.service.ts (5)
apps/api/src/billing/services/dedupe-signing-client/dedupe-signing-client.service.ts (1)
singleton(18-62)apps/api/src/billing/services/financial-stats/financial-stats.service.ts (1)
singleton(14-108)apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts (1)
singleton(20-192)packages/net/src/index.ts (1)
createNetConfig(6-8)apps/provider-proxy/src/services/feature-flags/feature-flags.service.ts (1)
FeatureFlagsService(3-14)
apps/api/src/chain/services/block-http/block-http.service.ts (1)
packages/http-sdk/src/block/block-http.service.ts (1)
BlockHttpService(13-23)
apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.spec.ts (2)
apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts (1)
BatchSigningClientService(38-296)apps/api/src/billing/lib/sync-signing-stargate-client/sync-signing-stargate-client.ts (1)
connectWithSigner(11-18)
apps/api/src/chain/services/block-http/block-http.service.spec.ts (1)
packages/http-sdk/src/block/block-http.service.ts (1)
BlockHttpService(13-23)
apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts (1)
apps/api/src/utils/env.ts (1)
env(3-27)
apps/api/src/billing/services/financial-stats/financial-stats.service.ts (1)
packages/http-sdk/src/cosmos/cosmos-http.service.ts (1)
CosmosHttpService(26-116)
apps/api/src/core/providers/http-sdk.provider.ts (8)
packages/http-sdk/src/balance/balance-http.service.ts (1)
BalanceHttpService(20-29)packages/http-sdk/src/authz/authz-http.service.ts (1)
AuthzHttpService(56-217)packages/http-sdk/src/block/block-http.service.ts (1)
BlockHttpService(13-23)packages/http-sdk/src/bid/bid-http.service.ts (1)
BidHttpService(53-63)packages/http-sdk/src/deployment/deployment-http.service.ts (1)
DeploymentHttpService(134-197)packages/http-sdk/src/lease/lease-http.service.ts (1)
LeaseHttpService(57-73)packages/http-sdk/src/provider/provider-http.service.ts (1)
ProviderHttpService(6-14)packages/http-sdk/src/cosmos/cosmos-http.service.ts (1)
CosmosHttpService(26-116)
⏰ 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). (5)
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
🔇 Additional comments (11)
apps/provider-proxy/src/services/feature-flags/feature-flags.service.ts (1)
3-14: LGTM: Simple, focused service and prior async method removedThe service is minimal, typed, and matches the PR goal. Also confirms removal of the previously questioned isEnabledAsync.
apps/api/src/billing/controllers/wallet/wallet.controller.spec.ts (1)
8-15: LGTM: New DI imports align with expanded controller dependenciesGood addition of repository/service imports and usage of jest-mock-extended for test-time DI. This keeps the controller resolution consistent with the new billing wiring.
apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts (1)
62-69: Constructor wiring: Good DI of ChainNetworkConfigServiceIntroduction of ChainNetworkConfigService via DI is correct and keeps endpoint resolution centralized.
apps/api/src/chain/services/block-http/block-http.service.ts (2)
1-1: Import alias change is appropriateRenaming to BaseBlockHttpService keeps intent clear and aligns with the HTTP SDK export.
9-13: LGTM: Constructor typing + explicit return typeConstructor uses the concrete SDK service via DI, and the explicit Promise return type on getCurrentHeight improves clarity. No issues.
apps/api/src/chain/services/block-http/block-http.service.spec.ts (1)
1-26: LGTM: Test follows our spec guidelines
- Uses jest-mock-extended (no jest.mock).
- setup() accepts a single inline-typed param, returns the SUT, and sits at the bottom of the root describe.
- No shared state. Clear, focused assertion.
Nice work.
apps/api/src/core/services/chain-network-config/chain-network-config.service.ts (1)
11-17: Initialize net config via feature flag — goodWiring createNetConfig with FeatureFlagsService and env proxy URLs centralizes behavior cleanly. No correctness issues spotted.
apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.spec.ts (2)
8-8: Good switch to type-only import for the DI token in testsUsing a type-only import for ChainNetworkConfigService in a spec is appropriate and keeps runtime clean.
14-14: Using the class name in describe keeps test names resilientdescribe(BatchSigningClientService.name, ...) is a nice touch for consistency.
apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts (1)
31-46: RPC URL resolution via ChainNetworkConfigService is correctValidating rpcUrl and wiring it into SigningStargateClient.connectWithSigner aligns this service with the new proxy-aware config.
apps/api/src/core/providers/http-sdk.provider.ts (1)
19-42: Centralizing baseURL via ChainNetworkConfigService is spot onEach HttpService now resolves its baseURL from ChainNetworkConfigService, which keeps proxy logic in one place. Explicit registrations read clearly and are easy to maintain.
apps/api/src/billing/services/financial-stats/financial-stats.service.ts
Outdated
Show resolved
Hide resolved
apps/api/src/billing/services/financial-stats/financial-stats.service.ts
Show resolved
Hide resolved
apps/provider-proxy/src/services/feature-flags/feature-flags.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/api/src/validator/services/validator/validator.service.ts (1)
41-49: Avoidanyand use a proper Axios type guard for 404 detection.Complies with the repo guideline to never use
anyand ensures safe narrowing.Apply this diff to the catch block:
- } catch (error: any) { - if (error.response?.status === 404) { + } catch (error: unknown) { + if (axios.isAxiosError(error) && error.response?.status === 404) { return null; } throw error; }And add this import at the top of the file:
import axios from "axios";
🧹 Nitpick comments (5)
apps/api/src/billing/services/financial-stats/financial-stats.service.ts (2)
56-60: Prefer Http SDK over raw axios for community pool; reuse configured baseURL + retries.Use CosmosHttpService.getCommunityPool() instead of direct axios. This keeps baseURL resolution centralized and benefits from retry policy in the SDK.
Apply this diff to refactor the method:
async getCommunityPoolUsdc() { - const apiNodeUrl = this.chainNetworkConfigService.getBaseAPIUrl(); - const communityPoolData = await axios.get<CosmosDistributionCommunityPoolResponse>(`${apiNodeUrl}/cosmos/distribution/v1beta1/community_pool`); - return parseFloat(communityPoolData.data.pool.find(x => x.denom === USDC_IBC_DENOMS.mainnetId)?.amount || "0"); + const pool = await this.cosmosHttpService.getCommunityPool(); + return parseFloat(pool.find(x => x.denom === USDC_IBC_DENOMS.mainnetId)?.amount || "0"); }Additionally, if you adopt the change above, remove the now-unused axios import:
- import axios from "axios";
27-32: Normalize units: methods labeled “Usdc” currently return micro-usdc (uusdc).getMasterWalletBalanceUsdc and getCommunityPoolUsdc return amounts in micro units, while getAkashPubProviderBalances converts to USDC. This inconsistency can cause downstream confusion.
Options:
- Normalize to USDC everywhere (divide by 1_000_000), or
- Keep micro-units but rename methods to reflect units (e.g., getMasterWalletBalanceUusdc).
If you choose normalization, minimal diffs on current code (pre-refactor) would be:
- return this.getWalletBalances(account.address, USDC_IBC_DENOMS.mainnetId); + const balanceMicro = await this.getWalletBalances(account.address, USDC_IBC_DENOMS.mainnetId); + return balanceMicro / 1_000_000;And for community pool (current axios-based version):
- return parseFloat(communityPoolData.data.pool.find(x => x.denom === USDC_IBC_DENOMS.mainnetId)?.amount || "0"); + return parseFloat(communityPoolData.data.pool.find(x => x.denom === USDC_IBC_DENOMS.mainnetId)?.amount || "0") / 1_000_000;If you adopt the SDK refactor suggested above, the community pool return would instead be:
- return parseFloat(pool.find(x => x.denom === USDC_IBC_DENOMS.mainnetId)?.amount || "0"); + return parseFloat(pool.find(x => x.denom === USDC_IBC_DENOMS.mainnetId)?.amount || "0") / 1_000_000;Please verify API consumers’ expectations before changing the unit to avoid breaking responses.
Also applies to: 56-60
apps/api/src/billing/services/balances/balances.service.ts (1)
61-61: Use explicit radix for parseInt; consider precision for large token amounts.
- Minimal fix: add radix 10 to avoid ambiguity and improve readability.
- Optional: these amounts can exceed Number.MAX_SAFE_INTEGER; consider BigInt/decimal handling to avoid precision loss.
Apply this minimal diff:
- return feeAllowance.allowance.spend_limit.reduce((acc, { denom, amount }) => (denom === "uakt" ? acc + parseInt(amount) : acc), 0); + return feeAllowance.allowance.spend_limit.reduce((acc, { denom, amount }) => (denom === "uakt" ? acc + Number.parseInt(amount, 10) : acc), 0);- return parseInt(depositDeploymentGrant.authorization.spend_limit.amount); + return Number.parseInt(depositDeploymentGrant.authorization.spend_limit.amount, 10);Optional follow-up: On Lines 88-94, summing escrow with
parseFloatmay lose precision for large values. If correctness is critical, sum using a big-number library (e.g., decimal.js) or BigInt in uakt, then convert for display.Also applies to: 72-72
apps/api/src/validator/services/validator/validator.service.ts (2)
27-34: Guard against divide-by-zero when computing votingPowerRatio.If the validator list is empty or
tokensparse to 0,totalVotingPoweris 0 and ratios becomeInfinity/NaN.Apply this diff:
.map((x, i) => ({ ...x, - votingPowerRatio: x.votingPower / totalVotingPower, + votingPowerRatio: totalVotingPower ? x.votingPower / totalVotingPower : 0, rank: i + 1, keybaseAvatarUrl: avatarMap.get(x.operatorAddress) ?? null }));
19-21: Use explicit radix with parseInt for clarity and consistency.Small readability and correctness win; avoids accidental octal interpretation.
Apply this diff:
- votingPower: parseInt(x.tokens), + votingPower: Number.parseInt(x.tokens, 10),- return { votingPower: parseInt(x.tokens), address: x.operator_address }; + return { votingPower: Number.parseInt(x.tokens, 10), address: x.operator_address };- votingPower: parseInt(cosmosResponse.validator.tokens), + votingPower: Number.parseInt(cosmosResponse.validator.tokens, 10),Also applies to: 55-56, 66-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/api/src/billing/services/balances/balances.service.ts(1 hunks)apps/api/src/billing/services/financial-stats/financial-stats.service.ts(3 hunks)apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts(4 hunks)apps/api/src/provider/controllers/provider-attributes-schema/provider-attributes-schema.controller.ts(1 hunks)apps/api/src/validator/services/validator/validator.service.ts(1 hunks)apps/provider-proxy/src/services/feature-flags/feature-flags.service.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/api/src/provider/controllers/provider-attributes-schema/provider-attributes-schema.controller.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/provider-proxy/src/services/feature-flags/feature-flags.service.ts
- apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/validator/services/validator/validator.service.tsapps/api/src/billing/services/balances/balances.service.tsapps/api/src/billing/services/financial-stats/financial-stats.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/validator/services/validator/validator.service.tsapps/api/src/billing/services/balances/balances.service.tsapps/api/src/billing/services/financial-stats/financial-stats.service.ts
🧠 Learnings (1)
📚 Learning: 2025-05-28T20:42:58.200Z
Learnt from: jzsfkzm
PR: akash-network/console#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/validator/services/validator/validator.service.ts
🧬 Code Graph Analysis (1)
apps/api/src/billing/services/financial-stats/financial-stats.service.ts (3)
apps/api/src/billing/services/dedupe-signing-client/dedupe-signing-client.service.ts (1)
singleton(18-62)apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts (1)
singleton(20-192)packages/http-sdk/src/cosmos/cosmos-http.service.ts (1)
CosmosHttpService(26-116)
⏰ 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/indexer) / validate-unsafe
- GitHub Check: validate (packages) / validate-unsafe
- GitHub Check: validate / validate-app
- 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: Validate local packages
🔇 Additional comments (5)
apps/api/src/billing/services/financial-stats/financial-stats.service.ts (3)
2-2: Good fix: value import for DI token kept, type-only for response.CosmosHttpService is correctly imported as a runtime value (needed for tsyringe), while CosmosDistributionCommunityPoolResponse is type-only. This resolves the DI issue flagged earlier.
11-11: Importing ChainNetworkConfigService aligns with proxy-based URL resolution.This is consistent with the new proxy-aware config flow. Nice.
19-21: Constructor DI looks correct for tsyringe.Both CosmosHttpService and ChainNetworkConfigService are runtime classes and will be emitted in design:paramtypes. Assuming they’re registered (or decorated with @singleton()), DI should resolve cleanly.
apps/api/src/billing/services/balances/balances.service.ts (1)
1-1: Type-only import is correct and improves tree-shaking.Good change:
DeploymentInfois used purely as a type, so importing it withtypeavoids emitting unused runtime code.apps/api/src/validator/services/validator/validator.service.ts (1)
1-1: Type-only import for RestCosmosStakingValidatorResponse is spot on.Importing as a type avoids unnecessary runtime code. Matches how it’s used below.
Summary by CodeRabbit
New Features
Bug Fixes
Chores