feat: load certificate from gRPC#2256
Conversation
WalkthroughThis PR migrates provider-proxy certificate retrieval from an HTTP/fetch-based REST call to a gRPC-based flow using Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Caller
participant ProviderSvc as ProviderService
participant Retry as RetryPolicy (cockatiel)
participant ChainSDK as Chain SDK Client
participant Grpc as gRPC Test/Node Server
participant Logger as Logger
Client->>ProviderSvc: getCertificate(providerAddress, serialNumber)
ProviderSvc->>ProviderSvc: build QueryCertificatesRequest
ProviderSvc->>Retry: execute(request) with ExponentialBackoff (max 3)
Retry->>ChainSDK: akash.cert.v1.getCertificates(request)
ChainSDK->>Grpc: gRPC call (protobuf payload)
alt Success
Grpc-->>ChainSDK: QueryCertificatesResponse (certificate bytes)
ChainSDK-->>Retry: response
Retry-->>ProviderSvc: certificates[]
ProviderSvc->>ProviderSvc: parse/filter, build X509Certificate
ProviderSvc-->>Client: X509Certificate | null
else Retriable Error (Unavailable/Internal)
Grpc-->>ChainSDK: gRPC error (code 14/13)
ChainSDK-->>Retry: error -> RetryPolicy triggers backoff & retry
Retry->>Logger: log retry attempt with context
Retry->>ChainSDK: retry request (up to 3 attempts)
else Non-retriable Error
Grpc-->>ChainSDK: error
ChainSDK-->>Retry: non-retriable -> fail
Retry-->>ProviderSvc: null / propagate
ProviderSvc->>Logger: log failure (providerAddress, serialNumber)
ProviderSvc-->>Client: null
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–80 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2256 +/- ##
==========================================
+ Coverage 47.60% 48.18% +0.57%
==========================================
Files 1036 1022 -14
Lines 29356 29009 -347
Branches 7607 7527 -80
==========================================
+ Hits 13976 13979 +3
+ Misses 14978 14574 -404
- Partials 402 456 +54
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
apps/provider-proxy/test/setup/grpcServer.ts (2)
9-48: Consider using a protobuf library for encoding.The manual protobuf encoding implementation works but is fragile and harder to maintain than using a protobuf library like
protobufjsor the generated types fromcosmjs-types. If the proto schema changes, these encoders will need manual updates.For test code, this approach is acceptable if it reduces dependencies, but ensure the encoding format matches the actual chain-sdk expectations.
97-110: Document the limitations of regex-based proto parsing.The regex-based extraction of
requestedOwnerandrequestedSerialfrom the raw proto buffer is heuristic and fragile. It relies on the binary data containing readable UTF-8 strings, which may not always be true.Potential issues:
- Could match false positives in other proto fields
- May break if proto encoding changes
- Assumes specific string patterns in binary data
Since this is test infrastructure and appears to work for the current use case, this is acceptable. However, add a comment documenting these limitations so future maintainers understand the tradeoffs.
+ // Heuristic parsing: extract owner (bech32) and serial from UTF-8 representation + // This is fragile but sufficient for test mocking. May break if proto encoding changes. if (requestData.length > 5) { const protoData = requestData.slice(5);apps/provider-proxy/src/services/ProviderService/ProviderService.ts (2)
25-33: Consider adding maxDelay to exponential backoff.The exponential backoff could grow indefinitely. Consider adding a
maxDelayparameter to cap the retry interval and prevent excessively long waits.private retryPolicy = retry( handleWhen(error => { return hasErrorCode(error) && RETRIABLE_ERROR_CODES.includes(error.code!); }), { maxAttempts: 3, - backoff: new ExponentialBackoff() + backoff: new ExponentialBackoff({ maxDelay: 5000 }) } );
45-45: Validate serialNumber format before BigInt conversion.If
serialNumbercontains invalid hex characters, the BigInt conversion will throw a SyntaxError. While the try-catch will prevent crashes, the error message won't clearly indicate the root cause.Add validation before the query:
async getCertificate(providerAddress: string, serialNumber: string): Promise<X509Certificate | null> { + if (!/^[0-9A-Fa-f]+$/.test(serialNumber)) { + this.logger?.error({ event: "INVALID_SERIAL_NUMBER", providerAddress, serialNumber }); + return null; + } + const queryParams: DeepPartial<QueryCertificatesRequest> = {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
apps/provider-proxy/env/.env(1 hunks)apps/provider-proxy/env/.env.sample(1 hunks)apps/provider-proxy/package.json(2 hunks)apps/provider-proxy/src/config/env.config.ts(1 hunks)apps/provider-proxy/src/container.ts(2 hunks)apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts(2 hunks)apps/provider-proxy/src/services/ProviderService/ProviderService.ts(1 hunks)apps/provider-proxy/test/functional/provider-proxy-http.spec.ts(26 hunks)apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts(16 hunks)apps/provider-proxy/test/setup/chainApiServer.ts(0 hunks)apps/provider-proxy/test/setup/grpcServer.ts(1 hunks)apps/provider-proxy/test/setup/providerServer.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/provider-proxy/test/setup/chainApiServer.ts
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-19T15:15:07.266Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.266Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/provider-proxy/test/functional/provider-proxy-ws.spec.tsapps/provider-proxy/package.jsonapps/provider-proxy/test/functional/provider-proxy-http.spec.tsapps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts
📚 Learning: 2025-09-25T20:34:55.117Z
Learnt from: jigar-arc10
Repo: akash-network/console PR: 1970
File: apps/provider-console/src/config/network.config.ts:17-28
Timestamp: 2025-09-25T20:34:55.117Z
Learning: In cosmos-kit integration, nodesUrl and versionUrl should point to Cosmos API endpoints (not console backend API endpoints) as cosmos-kit expects these to be Cosmos REST endpoints for proper functionality.
Applied to files:
apps/provider-proxy/env/.env.sampleapps/provider-proxy/env/.env
📚 Learning: 2025-09-04T04:27:50.638Z
Learnt from: stalniy
Repo: akash-network/console PR: 1868
File: apps/api/src/billing/services/managed-signer/managed-signer.service.ts:1-1
Timestamp: 2025-09-04T04:27:50.638Z
Learning: In the akash-network/console project, importing MsgCreateLease from "akashnetwork/akash-api/v1beta3" instead of v1beta4 is considered non-critical by the maintainers, likely due to backward compatibility or project-specific requirements.
Applied to files:
apps/provider-proxy/package.json
📚 Learning: 2025-11-12T09:03:40.132Z
Learnt from: stalniy
Repo: akash-network/console PR: 0
File: :0-0
Timestamp: 2025-11-12T09:03:40.132Z
Learning: For backend services (like the Indexer), prefer using createOtelLogger from "akashnetwork/logging/otel" to include OpenTelemetry trace context in logs.
Applied to files:
apps/provider-proxy/src/container.ts
📚 Learning: 2025-06-08T03:07:13.871Z
Learnt from: stalniy
Repo: akash-network/console PR: 1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts
🧬 Code graph analysis (5)
apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts (4)
apps/provider-proxy/test/setup/providerServer.ts (2)
stopProviderServer(73-75)startProviderServer(14-71)apps/provider-proxy/test/setup/proxyServer.ts (2)
stopServer(12-14)startServer(7-10)apps/provider-proxy/test/setup/grpcServer.ts (3)
stopGrpcServer(183-201)generateBech32(208-216)startGrpcServer(50-181)apps/provider-proxy/test/seeders/createX509CertPair.ts (1)
createX509CertPair(4-26)
apps/provider-proxy/test/functional/provider-proxy-http.spec.ts (4)
apps/provider-proxy/test/setup/proxyServer.ts (2)
stopServer(12-14)startServer(7-10)apps/provider-proxy/test/setup/providerServer.ts (2)
stopProviderServer(73-75)startProviderServer(14-71)apps/provider-proxy/test/setup/grpcServer.ts (3)
stopGrpcServer(183-201)generateBech32(208-216)startGrpcServer(50-181)apps/provider-proxy/test/seeders/createX509CertPair.ts (1)
createX509CertPair(4-26)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
packages/logging/src/services/logger/logger.service.ts (1)
error(141-143)
apps/provider-proxy/src/container.ts (1)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
ProviderService(24-73)
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (2)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
ProviderService(24-73)apps/provider-proxy/test/seeders/createX509CertPair.ts (2)
CertificateOptions(33-38)createX509CertPair(4-26)
⏰ 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). (14)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (29)
apps/provider-proxy/package.json (2)
35-38: LGTM! Dependencies align well with the gRPC migration.The new dependencies are appropriate:
cockatielfor resilient retry policiescosmjs-typesfor Cosmos protobuf message typeslongfor handling 64-bit integers in gRPC responses
25-25: I'll now search for security advisories on this package to complete the verification.Let me search more specifically for this npm package's vulnerability status.
Alpha version is available; no specific security advisories found for this package.
The version @akashnetwork/chain-sdk@1.0.0-alpha.18 exists in the npm registry. However, no known CVEs or security advisories were found for this specific package version. The original concern about production use of an alpha version is valid from a stability perspective, but this is a design/maturity decision rather than a security issue.
If the project requires this version for compatibility with the Akash Network chain, the alpha designation should be acceptable as long as the team is aware of potential breaking changes in future updates. Consider monitoring the package for updates to stable releases when available.
apps/provider-proxy/env/.env (1)
1-1: LGTM! Valid gRPC endpoint configuration.The GRPC_NODE_URL correctly uses HTTPS scheme (gRPC over TLS/HTTP2) with standard port 443.
apps/provider-proxy/env/.env.sample (1)
1-1: LGTM! Sample configuration points to sandbox environment.The sample GRPC_NODE_URL correctly points to the sandbox-2 environment with appropriate gRPC port 9090.
apps/provider-proxy/test/setup/providerServer.ts (1)
10-10: LGTM! Import path correctly updated for gRPC test infrastructure.The import of
generateBech32is correctly moved from the old chain API server module to the new gRPC server module.apps/provider-proxy/src/config/env.config.ts (1)
4-4: LGTM! Config schema correctly updated for gRPC.The environment variable is properly renamed from
REST_API_NODE_URLtoGRPC_NODE_URLwith URL validation maintained. Note that this is a breaking change requiring environment variable updates in deployment configurations.apps/provider-proxy/src/container.ts (2)
1-1: LGTM! Correct import for the chain SDK client.The
createChainNodeSDKfunction is properly imported from the upgraded chain-sdk package.
17-24: LGTM! Service initialization correctly migrated to gRPC-based client.The ProviderService is now initialized with a ChainNodeSDK client instead of REST URL and fetch function. This aligns perfectly with the PR objective to migrate from HTTP to gRPC for reduced latency and better type safety.
The configuration correctly:
- Creates a ChainNodeSDK client with the gRPC endpoint
- Removes the fetch dependency
- Maintains logger integration
apps/provider-proxy/test/functional/provider-proxy-http.spec.ts (6)
8-8: LGTM! Test imports correctly updated for gRPC test infrastructure.The imports are correctly updated to use the new gRPC server utilities (
startGrpcServer,stopGrpcServer,generateBech32) replacing the old chain API server.
18-18: LGTM! Test cleanup correctly stops all servers including gRPC.The afterEach hook properly cleans up all test servers including the new gRPC server.
25-27: LGTM! Test correctly uses gRPC server for certificate validation.The test properly:
- Starts the gRPC server with the valid certificate
- Configures the proxy with the gRPC endpoint
- Validates the proxied request succeeds
72-89: LGTM! Excellent test for cached certificate fallback.This test validates that the proxy can continue operating with cached certificates even when the gRPC server becomes unavailable. This is important for resilience.
225-256: LGTM! Great test coverage for retry logic on gRPC unavailability.This test validates the retry mechanism by:
- Starting a gRPC server to reserve the port
- Stopping it to simulate unavailability
- Making a request (which will retry)
- Restarting the server on the same port mid-retry
- Confirming the request succeeds
This ensures the retry policy works correctly with transient network issues.
258-291: LGTM! Excellent test for gRPC error code 14 retry logic.This test validates that the proxy correctly retries when receiving gRPC error code 14 (UNAVAILABLE), which is a retriable error. The test uses an interceptor to simulate one failure before succeeding, confirming the retry policy works as expected.
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (9)
1-2: LGTM! Correct imports for gRPC-based testing.The imports correctly bring in
SDKErrorfor testing error scenarios and thecreateChainNodeSDKtype for proper mocking.
8-8: LGTM! Type extraction provides proper typing for mocks.Extracting the
ChainNodeSDKtype from the return type ofcreateChainNodeSDKensures type safety in test mocks.
12-30: LGTM! Comprehensive test for no certificate found scenario.The test correctly:
- Mocks an empty certificate response
- Verifies the query parameters (state, owner, serial, pagination)
- Confirms null is returned when no certificate exists
The serial number conversion from hex to decimal is properly tested (177831BE7F249E66 → 1691156354324274790).
32-40: LGTM! Test validates single certificate retrieval.The test correctly verifies that when exactly one certificate is found, it's returned with the correct serial number.
42-50: LGTM! Good edge case coverage for multiple certificates.This test validates that when multiple certificates are returned (which shouldn't happen with limit=1, but could indicate a data issue), the service returns null for safety.
52-63: LGTM! Excellent test for retry success after transient failures.This test validates that the retry policy:
- Retries on gRPC error code 14 (UNAVAILABLE)
- Eventually succeeds after transient failures
- Makes exactly 3 calls (2 failures + 1 success)
74-81: LGTM! Correct behavior for non-retriable errors.This test validates that errors other than code 14 (e.g., code 12 for UNIMPLEMENTED) are not retried, which is the correct behavior. The service fails fast and returns null immediately.
124-134: LGTM! Mock setup correctly reflects ChainNodeSDK structure.The setup function properly mocks the nested structure of the ChainNodeSDK (
akash.cert.v1.getCertificates), allowing individual test cases to override thegetCertificatesimplementation.
136-138: LGTM! Certificate builder updated to match gRPC response format.The certificate is now returned as a Buffer (from
.toJSON()) instead of a base64 string, which aligns with the actual protobuf/gRPC response format.apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts (1)
8-307: LGTM! Test infrastructure migration is consistent.All changes correctly migrate from the Chain API server to the gRPC server infrastructure. The test logic and assertions remain unchanged, which is appropriate.
apps/provider-proxy/test/setup/grpcServer.ts (2)
113-123: LGTM! Certificate matching logic is correct.The serial number conversion from hex to decimal (line 116) correctly matches the reverse conversion in
ProviderService.ts(line 45), and the optional matching logic handles null filters appropriately.
125-161: LGTM! gRPC response handling is properly implemented.The gRPC framing (5-byte header + message) and response construction correctly handles both empty results and certificate responses with appropriate grpc-status codes.
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (3)
62-66: LGTM! Retry policy correctly wraps the SDK call.The type cast from
DeepPartialtoQueryCertificatesRequestis necessary and safe since all required fields are provided in the query parameters.
52-59: LGTM! Error handling with appropriate logging.The error handling correctly logs failures with context (providerAddress, serialNumber, code) and returns null, which is appropriate for a certificate lookup operation. The retry policy already handles transient failures before reaching this catch block.
1-2: Verified: Import path is consistent with established codebase patterns.The import path from
@akashnetwork/chain-sdk/private-types/akash.v1is widely used throughout the codebase across 17+ files (notifications, deploy-web, api, stats-web, and provider-proxy apps), indicating it is the standard and accepted way to access these types in this project. Despite extensive usage, there is no evidence of breakage. The project uses pre-release version1.0.0-alpha.12, which is context appropriate for accepting this import pattern.
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts
Show resolved
Hide resolved
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
Outdated
Show resolved
Hide resolved
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
Outdated
Show resolved
Hide resolved
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
Outdated
Show resolved
Hide resolved
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
Outdated
Show resolved
Hide resolved
jzsfkzm
force-pushed
the
features/2188-certificates-from-grpc
branch
2 times, most recently
from
7787709 to
bd4e7ef
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (1)
64-71: Inconsistent retry count expectation needs resolution.The test expects 4 calls on line 70, but with
maxAttempts: 3in the retry policy (line 28 of ProviderService.ts), cockatiel will make only 3 total attempts (1 initial + 2 retries). A previous review comment indicated this should be changed from 4 to 3 and was marked "Addressed", but the code still shows 4.Please verify the correct behavior and update accordingly:
- expect(getCertificates).toHaveBeenCalledTimes(4); + expect(getCertificates).toHaveBeenCalledTimes(3);Also consider updating the test description for clarity:
- it("retries getCertificates() request 3 times and then fails in case of grpc-status=14", async () => { + it("attempts getCertificates() 3 times (maxAttempts) and then fails in case of grpc-status=14", async () => {
🧹 Nitpick comments (3)
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (1)
123-133: Consider extracting ChainNodeSDK type to avoid repetition.The ChainNodeSDK type alias could be defined once at module level (line 7) and reused here, improving maintainability.
function setup({ getCertificates }: { getCertificates?: ChainNodeSDK["akash"]["cert"]["v1"]["getCertificates"] } = {}) {apps/provider-proxy/test/setup/chainApiServer.ts (1)
9-48: Manual protobuf encoding is functional but undocumented.The manual protobuf encoding functions implement the necessary wire format for gRPC testing. Consider adding a comment explaining why manual encoding is used instead of a protobuf library.
// Manual protobuf encoding for test server to avoid heavy protobuf.js dependency // Implements minimal wire format needed for Certificates response function encodeVarint(value: number): Buffer {apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
64-68: Consider removing type cast if QueryInput types are correct.The cast
as QueryCertificatesRequeston line 66 suggests a potential type mismatch. IfQueryInput<QueryCertificatesRequest>should be compatible withQueryCertificatesRequest, consider investigating why the cast is needed.#!/bin/bash # Check the QueryInput type definition in chain-sdk ast-grep --pattern 'type QueryInput<$T> = $$$'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
apps/provider-proxy/env/.env(1 hunks)apps/provider-proxy/env/.env.sample(1 hunks)apps/provider-proxy/package.json(3 hunks)apps/provider-proxy/src/config/env.config.ts(1 hunks)apps/provider-proxy/src/container.ts(2 hunks)apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts(2 hunks)apps/provider-proxy/src/services/ProviderService/ProviderService.ts(1 hunks)apps/provider-proxy/test/functional/provider-proxy-http.spec.ts(26 hunks)apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts(10 hunks)apps/provider-proxy/test/setup/chainApiServer.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/provider-proxy/env/.env
- apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts
- apps/provider-proxy/env/.env.sample
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-12T09:03:40.132Z
Learnt from: stalniy
Repo: akash-network/console PR: 0
File: :0-0
Timestamp: 2025-11-12T09:03:40.132Z
Learning: For backend services (like the Indexer), prefer using createOtelLogger from "akashnetwork/logging/otel" to include OpenTelemetry trace context in logs.
Applied to files:
apps/provider-proxy/src/container.ts
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/provider-proxy/package.jsonapps/provider-proxy/test/functional/provider-proxy-http.spec.tsapps/provider-proxy/test/setup/chainApiServer.tsapps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts
📚 Learning: 2025-09-04T04:27:50.638Z
Learnt from: stalniy
Repo: akash-network/console PR: 1868
File: apps/api/src/billing/services/managed-signer/managed-signer.service.ts:1-1
Timestamp: 2025-09-04T04:27:50.638Z
Learning: In the akash-network/console project, importing MsgCreateLease from "akashnetwork/akash-api/v1beta3" instead of v1beta4 is considered non-critical by the maintainers, likely due to backward compatibility or project-specific requirements.
Applied to files:
apps/provider-proxy/package.json
📚 Learning: 2025-06-08T03:07:13.871Z
Learnt from: stalniy
Repo: akash-network/console PR: 1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts
🧬 Code graph analysis (4)
apps/provider-proxy/src/container.ts (1)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
ProviderService(22-75)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
packages/logging/src/services/logger/logger.service.ts (1)
error(141-143)
apps/provider-proxy/test/functional/provider-proxy-http.spec.ts (4)
apps/provider-proxy/test/setup/proxyServer.ts (2)
stopServer(12-14)startServer(7-10)apps/provider-proxy/test/setup/providerServer.ts (2)
stopProviderServer(73-75)startProviderServer(14-71)apps/provider-proxy/test/setup/chainApiServer.ts (3)
stopChainApiServer(183-201)generateBech32(208-216)startChainApiServer(50-181)apps/provider-proxy/test/seeders/createX509CertPair.ts (1)
createX509CertPair(4-26)
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (2)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
ProviderService(22-75)apps/provider-proxy/test/seeders/createX509CertPair.ts (2)
CertificateOptions(33-38)createX509CertPair(4-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
🔇 Additional comments (10)
apps/provider-proxy/src/config/env.config.ts (1)
4-4: LGTM! Clean environment variable migration.The rename from REST_API_NODE_URL to GRPC_NODE_URL properly reflects the architectural shift to gRPC-based communication.
apps/provider-proxy/test/functional/provider-proxy-http.spec.ts (2)
8-8: LGTM! Consistent test migration to gRPC terminology.All test scaffolding properly updated to use gRPC naming conventions (grpcServer, stopChainApiServer, GRPC_NODE_URL) while preserving test logic.
Also applies to: 18-18, 25-27, 50-52
265-274: No issues found—callback signature correctly aligns with the updated interface.The interface definition in
chainApiServer.tsspecifiesinterceptRequest?: () => boolean, which precisely matches the test implementation at lines 265-274 where the callback takes no parameters and returns a boolean value. The change is correct.apps/provider-proxy/src/container.ts (1)
17-24: LGTM! Clean Chain SDK integration.The container properly instantiates the Chain SDK client with the gRPC endpoint and passes it to ProviderService, replacing the previous REST-based approach.
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (1)
11-29: LGTM! Test properly validates empty certificate response.The test correctly mocks an empty certificates array and verifies null is returned, with proper query parameter construction including hex-to-decimal serial conversion.
apps/provider-proxy/test/setup/chainApiServer.ts (2)
50-181: LGTM! HTTP/2 server implementation for gRPC testing.The HTTP/2-based test server properly implements gRPC-style framing and certificate lookup logic, with appropriate session cleanup and error handling.
203-206: Signature change is properly integrated across all call sites.Verification confirms the
interceptRequestsignature change from(req, res) => booleanto() => booleanhas been fully applied:
- Call site (line 65): Invoked as
options?.interceptRequest?.()with no arguments- Implementation (line 267): Defined as
interceptRequest() { ... }with no parameters- Interface (line 205): Typed as
interceptRequest?: () => boolean- Scope: Used only in test code; no production usages require updating
All locations are consistent and correctly updated.
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (2)
23-31: LGTM! Well-structured retry policy with exponential backoff.The retry configuration properly handles transient gRPC errors (internal and unavailable) with bounded retries and exponential backoff.
38-62: LGTM! Robust certificate fetching with proper error handling.The implementation correctly:
- Constructs the query with hex-to-decimal serial conversion
- Uses safe optional chaining for certificate extraction
- Logs errors with full context
- Returns null on any failure (defensive)
apps/provider-proxy/package.json (1)
25-25: Review verification inconclusive—manual check recommended.The version jump from alpha.12 to alpha.18 (6 intermediate versions) is real and confirmed in provider-proxy/package.json. The codebase imports
JwtTokenManager,createChainNodeSDK, and type definitions from chain-sdk, but definitive breaking change documentation for alpha.13→alpha.18 could not be retrieved.The package is already at alpha.18, suggesting either migration is complete or not yet tested. Verify that:
- Intermediate version breaking changes (if any) were handled
- Test suite passes with alpha.18
- No API incompatibilities remain in ProviderService.ts, container.ts, or schema.ts
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
Outdated
Show resolved
Hide resolved
jzsfkzm
force-pushed
the
features/2188-certificates-from-grpc
branch
from
bd4e7ef to
d54cd43
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (1)
64-71: Retry-count assertion doesn’t match cockatiel’smaxAttemptssemanticsIn
it("retries getCertificates() request 3 times and then fails in case of grpc-status=14"),ProviderServiceis configured withmaxAttempts: 3, and cockatiel interprets that as the total number of attempts (initial call included). So for a permanentSDKError("unavailable", 14)you should see 3 calls, not 4.The current assertion:
expect(getCertificates).toHaveBeenCalledTimes(4);is inconsistent with that and with the previous test that correctly expects 3 calls after two failures and a success. I’d change it to:
- expect(getCertificates).toHaveBeenCalledTimes(4); + expect(getCertificates).toHaveBeenCalledTimes(3);(and optionally tweak the test title to say “retries up to maxAttempts and then fails” to avoid confusion about “3 times” vs “3 total attempts”).
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
9-20: Limit retries to UNAVAILABLE (14), avoid auto‑retrying INTERNAL (13).Including gRPC INTERNAL (13) in
RETRIABLE_ERROR_CODESrisks hiding real server/implementation bugs; typically only UNAVAILABLE (14) is treated as transient and retried. I’d strongly suggest restricting retries to 14 here and investigating why 13 is being observed when the service is down instead of broadening the retriable set.A minimal change would be:
-const RETRIABLE_ERROR_CODES = [ - 13, // internal - 14 // unavailable -]; +const RETRIABLE_ERROR_CODES = [ + 14 // unavailable +];You may also want to additionally guard that
error.codeis a number before checkingincludesto avoid surprising matches from non‑gRPC errors.
🧹 Nitpick comments (3)
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (1)
1-8: Prefer a direct ChainNodeSDK type instead ofimport type createChainNodeSDK+ReturnTypeHere you import
createChainNodeSDKas a type and then useReturnType<typeof createChainNodeSDK>to defineChainNodeSDK. That’s a bit round‑about and, depending on TS configuration, mixingimport typewithtypeofon the same symbol can be brittle.If
@akashnetwork/chain-sdkexposes aChainNodeSDKtype, I’d strongly prefer:import type { ChainNodeSDK } from "@akashnetwork/chain-sdk";and drop the
ReturnTypeindirection. If it doesn’t, consider a normal value import ofcreateChainNodeSDKfor the type query instead ofimport type.Also applies to: 123-133
apps/provider-proxy/test/setup/chainApiServer.ts (1)
6-8: Optionally clearsessionson shutdown to avoid stale referencesBoth the
closefunction returned fromstartChainApiServerandstopChainApiServerdestroy every entry in the globalsessionsarray but never reset the array itself. Over many test runs this can leave a growing list of staleServerHttp2Sessionreferences.Not a functional bug in this test harness, but you could add
sessions.length = 0;after thesessions.forEach(session => session.destroy());blocks to keep the state clean between server lifecycles.Also applies to: 170-177, 188-200
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
38-48: Tighten serial parsing error handling and alignQueryInputtyping with the SDK.Two small follow‑ups to consider:
Serial parsing can throw before the
tryblock.
BigInt(\0x${serialNumber}`)will throw on malformed input, and that happens before yourtry/catch, so such errors won’t be logged asCERTIFICATE_FETCH_ERROR. Parsing the serial inside atry/catch(and logging/returningnull` on failure) would keep error reporting consistent and prevent unexpected rejections from bad input.Avoid the
as QueryCertificatesRequestcast infetchCertificate.
fetchCertificateacceptsQueryInput<QueryCertificatesRequest>but then casts toQueryCertificatesRequestfor the SDK call:return await this.chainSdkClient.akash.cert.v1.getCertificates( getCertificatesParams as QueryCertificatesRequest );To preserve the stronger typings you get from
QueryInput, it’s better to align this with the actualchain-sdksignature and drop the cast. For example, ifgetCertificatesindeed expects aQueryInput<QueryCertificatesRequest>, you can do:
- private async fetchCertificate(getCertificatesParams: QueryInput): Promise {
- return this.retryPolicy.execute(async () => {
return await this.chainSdkClient.akash.cert.v1.getCertificates(getCertificatesParams as QueryCertificatesRequest);- });
- }
- private async fetchCertificate(
- getCertificatesParams: QueryInput
- ): Promise {
- return this.retryPolicy.execute(async () => {
return await this.chainSdkClient.akash.cert.v1.getCertificates(getCertificatesParams);- });
- }
If the SDK method instead truly expects a plain `QueryCertificatesRequest`, consider changing the parameter type of `fetchCertificate` to that and building the exact shape there, rather than casting.These are mainly about robustness and type‑safety; the overall flow and certificate extraction logic otherwise look good.
Also applies to: 50-62, 64-67
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
apps/provider-proxy/env/.env(1 hunks)apps/provider-proxy/env/.env.sample(1 hunks)apps/provider-proxy/package.json(3 hunks)apps/provider-proxy/src/config/env.config.ts(1 hunks)apps/provider-proxy/src/container.ts(2 hunks)apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts(2 hunks)apps/provider-proxy/src/services/ProviderService/ProviderService.ts(1 hunks)apps/provider-proxy/test/functional/provider-proxy-http.spec.ts(26 hunks)apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts(10 hunks)apps/provider-proxy/test/setup/chainApiServer.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/provider-proxy/env/.env.sample
- apps/provider-proxy/src/config/env.config.ts
- apps/provider-proxy/env/.env
- apps/provider-proxy/test/functional/provider-proxy-http.spec.ts
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/provider-proxy/package.jsonapps/provider-proxy/test/setup/chainApiServer.tsapps/provider-proxy/test/functional/provider-proxy-ws.spec.tsapps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts
📚 Learning: 2025-09-04T04:27:50.638Z
Learnt from: stalniy
Repo: akash-network/console PR: 1868
File: apps/api/src/billing/services/managed-signer/managed-signer.service.ts:1-1
Timestamp: 2025-09-04T04:27:50.638Z
Learning: In the akash-network/console project, importing MsgCreateLease from "akashnetwork/akash-api/v1beta3" instead of v1beta4 is considered non-critical by the maintainers, likely due to backward compatibility or project-specific requirements.
Applied to files:
apps/provider-proxy/package.json
📚 Learning: 2025-10-31T11:26:42.138Z
Learnt from: stalniy
Repo: akash-network/console PR: 2138
File: apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts:379-382
Timestamp: 2025-10-31T11:26:42.138Z
Learning: In TypeScript/JavaScript, the pattern of checking a cached value and then performing an async operation to fetch it without proper synchronization is race condition unsafe:
```typescript
private async getAddress() {
if (!this.address) {
this.address = await this.wallet.getFirstAddress();
}
return this.address;
}
```
Multiple concurrent calls can all pass the `if (!this.address)` check before any of them sets the value, leading to duplicate async operations. This should be flagged as a race condition. Proper synchronization (mutex, atomic promise caching, or guaranteed single-threaded execution) is required.
Applied to files:
apps/provider-proxy/test/setup/chainApiServer.ts
📚 Learning: 2025-09-04T04:27:40.858Z
Learnt from: stalniy
Repo: akash-network/console PR: 1868
File: apps/api/src/billing/services/managed-signer/managed-signer.service.ts:98-107
Timestamp: 2025-09-04T04:27:40.858Z
Learning: In the Akash protocol, when processing MsgCreateLease messages, the bidId.dseq field is always required and guaranteed to be present at runtime, even though TypeScript types may suggest it could be undefined. The non-null assertion operator (!) is appropriate to use in this context.
Applied to files:
apps/provider-proxy/test/setup/chainApiServer.ts
📚 Learning: 2025-11-12T09:03:40.132Z
Learnt from: stalniy
Repo: akash-network/console PR: 0
File: :0-0
Timestamp: 2025-11-12T09:03:40.132Z
Learning: For backend services (like the Indexer), prefer using createOtelLogger from "akashnetwork/logging/otel" to include OpenTelemetry trace context in logs.
Applied to files:
apps/provider-proxy/src/container.ts
📚 Learning: 2025-06-08T03:07:13.871Z
Learnt from: stalniy
Repo: akash-network/console PR: 1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts
🧬 Code graph analysis (4)
apps/provider-proxy/src/container.ts (1)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
ProviderService(22-75)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
packages/logging/src/services/logger/logger.service.ts (1)
error(141-143)
apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts (3)
apps/provider-proxy/test/setup/proxyServer.ts (2)
stopServer(12-14)startServer(7-10)apps/provider-proxy/test/setup/providerServer.ts (1)
stopProviderServer(73-75)apps/provider-proxy/test/setup/chainApiServer.ts (1)
stopChainApiServer(183-201)
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (3)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
ProviderService(22-75)packages/http-sdk/src/certificates/certificates.service.ts (1)
getCertificates(25-37)apps/provider-proxy/test/seeders/createX509CertPair.ts (2)
CertificateOptions(33-38)createX509CertPair(4-26)
⏰ 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). (14)
- GitHub Check: log-collector-ci
- GitHub Check: validate (packages) / validate-unsafe
- 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 / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (5)
apps/provider-proxy/package.json (1)
25-25: Deps bump aligns with new ChainNodeSDK + retry plumbingBumping
@akashnetwork/chain-sdkand addingcockatiel/ts-nodelines up with the new gRPC-basedProviderServiceand retry logic; nothing concerning here.Also applies to: 35-35, 65-65
apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts (1)
8-15: Teardown and GRPC wiring look consistentUsing
afterEach(async () => await Promise.all([...]))to stop proxy, provider, and chain API servers, plus switchingstartServertoGRPC_NODE_URL: chainServer.url, matches the new ChainNodeSDK/gRPC flow and keeps the tests’ lifecycle symmetrical. Looks good.Also applies to: 37-37, 68-68, 88-88, 116-116, 139-139, 167-167, 194-195, 242-242, 305-305
apps/provider-proxy/src/container.ts (1)
1-1: ProviderService wiring with ChainNodeSDK looks correctConstructing
ProviderServicewithcreateChainNodeSDK({ query: { baseUrl: appConfig.GRPC_NODE_URL } })fits the new gRPC-based, read-only usage and centralizes the SDK client in the container; I don’t see issues here. It’s only worth double‑checking against the current chain‑sdk docs thatquery.baseUrlis indeed the expected knob for the HTTP/2 gRPC endpoint in1.0.0-alpha.18.Also applies to: 17-24
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (1)
135-137: Ignore this review comment—it is based on incorrect assumptions about the Node.js crypto API.The review incorrectly claims that
X509Certificate.toJSON()returns "a plain object." Per Node.js documentation,toJSON()actually returns a string containing the PEM-encoded certificate. The current implementation is correct:
createX509CertPair(params).cert.toJSON()returns a PEM stringBuffer.from(pem_string)successfully creates a buffer from the UTF-8 stringnew X509Certificate(cert_buffer)inProviderService.getCertificatesuccessfully parses itThe production test setup in
providerServer.ts(line 19) further confirms this works: it usescert: certPair.cert.toJSON()directly inhttps.ServerOptions. The code is functioning as intended.Likely an incorrect or invalid review comment.
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
23-31: Retry policy integration aroundgetCertificateslooks appropriate.Using a shared
retryPolicywithmaxAttempts: 3and exponential backoff around the read‑onlygetCertificatescall is a good fit here; the encapsulation infetchCertificatekeepsgetCertificateclean and focused, and I don’t see functional issues in how the policy is wired and executed.Also applies to: 64-67
jzsfkzm
force-pushed
the
features/2188-certificates-from-grpc
branch
from
d54cd43 to
0c502e8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
9-12: Remove error code 13 from RETRIABLE_ERROR_CODES.gRPC INTERNAL (code 13) represents serious invariant/implementation errors that should not be automatically retried. Only UNAVAILABLE (code 14) is the canonical transient code for retries per gRPC best practices.
const RETRIABLE_ERROR_CODES = [ - 13, // internal 14 // unavailable ];
🧹 Nitpick comments (2)
apps/provider-proxy/test/setup/chainApiServer.ts (2)
7-8: Sessions array is never cleared after server shutdown.The
sessionsarray accumulatesServerHttp2Sessionreferences that are destroyed but never removed. If tests restart the server multiple times within the same process, the array grows with stale references.Consider clearing the array after destroying sessions:
return new Promise<void>((resolve, reject) => { sessions.forEach(session => { session.destroy(); }); + sessions.length = 0; grpcServer?.close(error => {
85-94: Duplicate session cleanup logic.The session destruction logic here duplicates what's in
stopChainApiServer(lines 106-108). Consider extracting to a shared helper to maintain consistency.+function destroyAllSessions() { + sessions.forEach(session => session.destroy()); + sessions.length = 0; +} + // In close callback: - close: () => - new Promise<void>((resolve, reject) => { - sessions.forEach(session => { - session.destroy(); - }); - - server.close(error => { - error ? reject(error) : resolve(); - }); - }) + close: () => + new Promise<void>((resolve, reject) => { + destroyAllSessions(); + server.close(error => { + error ? reject(error) : resolve(); + }); + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
apps/provider-proxy/env/.env.mainnet(1 hunks)apps/provider-proxy/env/.env.sample(1 hunks)apps/provider-proxy/env/.env.sandbox(1 hunks)apps/provider-proxy/package.json(2 hunks)apps/provider-proxy/src/config/env.config.ts(1 hunks)apps/provider-proxy/src/container.ts(2 hunks)apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts(2 hunks)apps/provider-proxy/src/services/ProviderService/ProviderService.ts(1 hunks)apps/provider-proxy/test/functional/provider-proxy-http.spec.ts(26 hunks)apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts(10 hunks)apps/provider-proxy/test/setup/chainApiServer.ts(1 hunks)apps/provider-proxy/test/setup/proxyServer.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/provider-proxy/env/.env.sample
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/provider-proxy/src/container.tsapps/provider-proxy/test/functional/provider-proxy-http.spec.tsapps/provider-proxy/test/setup/proxyServer.tsapps/provider-proxy/src/config/env.config.tsapps/provider-proxy/test/functional/provider-proxy-ws.spec.tsapps/provider-proxy/src/services/ProviderService/ProviderService.tsapps/provider-proxy/test/setup/chainApiServer.tsapps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/provider-proxy/test/functional/provider-proxy-http.spec.tsapps/provider-proxy/test/functional/provider-proxy-ws.spec.tsapps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts
🧠 Learnings (8)
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/provider-proxy/package.jsonapps/provider-proxy/test/functional/provider-proxy-http.spec.tsapps/provider-proxy/test/functional/provider-proxy-ws.spec.tsapps/provider-proxy/test/setup/chainApiServer.tsapps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts
📚 Learning: 2025-09-04T04:27:50.638Z
Learnt from: stalniy
Repo: akash-network/console PR: 1868
File: apps/api/src/billing/services/managed-signer/managed-signer.service.ts:1-1
Timestamp: 2025-09-04T04:27:50.638Z
Learning: In the akash-network/console project, importing MsgCreateLease from "akashnetwork/akash-api/v1beta3" instead of v1beta4 is considered non-critical by the maintainers, likely due to backward compatibility or project-specific requirements.
Applied to files:
apps/provider-proxy/package.json
📚 Learning: 2025-09-25T20:34:55.117Z
Learnt from: jigar-arc10
Repo: akash-network/console PR: 1970
File: apps/provider-console/src/config/network.config.ts:17-28
Timestamp: 2025-09-25T20:34:55.117Z
Learning: In cosmos-kit integration, nodesUrl and versionUrl should point to Cosmos API endpoints (not console backend API endpoints) as cosmos-kit expects these to be Cosmos REST endpoints for proper functionality.
Applied to files:
apps/provider-proxy/env/.env.sandboxapps/provider-proxy/env/.env.mainnet
📚 Learning: 2025-11-12T09:03:40.132Z
Learnt from: stalniy
Repo: akash-network/console PR: 0
File: :0-0
Timestamp: 2025-11-12T09:03:40.132Z
Learning: For backend services (like the Indexer), prefer using createOtelLogger from "akashnetwork/logging/otel" to include OpenTelemetry trace context in logs.
Applied to files:
apps/provider-proxy/src/container.ts
📚 Learning: 2025-11-25T17:45:58.258Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/test-descriptions.mdc:0-0
Timestamp: 2025-11-25T17:45:58.258Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `<Subject>.name` in the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Applied to files:
apps/provider-proxy/test/functional/provider-proxy-http.spec.tsapps/provider-proxy/test/functional/provider-proxy-ws.spec.ts
📚 Learning: 2025-11-25T17:45:49.180Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-11-25T17:45:49.180Z
Learning: Applies to {apps/deploy-web,apps/provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations
Applied to files:
apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts
📚 Learning: 2025-10-31T11:26:42.138Z
Learnt from: stalniy
Repo: akash-network/console PR: 2138
File: apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts:379-382
Timestamp: 2025-10-31T11:26:42.138Z
Learning: In TypeScript/JavaScript, the pattern of checking a cached value and then performing an async operation to fetch it without proper synchronization is race condition unsafe:
```typescript
private async getAddress() {
if (!this.address) {
this.address = await this.wallet.getFirstAddress();
}
return this.address;
}
```
Multiple concurrent calls can all pass the `if (!this.address)` check before any of them sets the value, leading to duplicate async operations. This should be flagged as a race condition. Proper synchronization (mutex, atomic promise caching, or guaranteed single-threaded execution) is required.
Applied to files:
apps/provider-proxy/test/setup/chainApiServer.ts
📚 Learning: 2025-06-08T03:07:13.871Z
Learnt from: stalniy
Repo: akash-network/console PR: 1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts
🧬 Code graph analysis (5)
apps/provider-proxy/test/functional/provider-proxy-http.spec.ts (4)
apps/provider-proxy/test/setup/proxyServer.ts (2)
stopServer(12-14)startServer(7-10)apps/provider-proxy/test/setup/providerServer.ts (2)
stopProviderServer(73-75)startProviderServer(14-71)apps/provider-proxy/test/setup/chainApiServer.ts (3)
stopChainApiServer(100-118)generateBech32(125-133)startChainApiServer(10-98)apps/provider-proxy/test/seeders/createX509CertPair.ts (1)
createX509CertPair(4-26)
apps/provider-proxy/test/setup/proxyServer.ts (2)
apps/api/src/core/services/start-server/start-server.ts (1)
startServer(18-64)apps/provider-proxy/src/config/env.config.ts (1)
AppConfig(8-8)
apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts (3)
apps/provider-proxy/test/setup/proxyServer.ts (2)
stopServer(12-14)startServer(7-10)apps/provider-proxy/test/setup/providerServer.ts (1)
stopProviderServer(73-75)apps/provider-proxy/test/setup/chainApiServer.ts (1)
stopChainApiServer(100-118)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
packages/logging/src/services/logger/logger.service.ts (1)
error(141-143)
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (3)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
ProviderService(22-75)packages/http-sdk/src/certificates/certificates.service.ts (1)
getCertificates(25-37)apps/provider-proxy/test/seeders/createX509CertPair.ts (2)
CertificateOptions(33-38)createX509CertPair(4-26)
⏰ 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). (16)
- GitHub Check: validate (apps/notifications) / validate-unsafe
- GitHub Check: validate (apps/provider-console) / validate-unsafe
- GitHub Check: validate (apps/log-collector) / validate-unsafe
- GitHub Check: validate (apps/indexer) / validate-unsafe
- 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 / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (27)
apps/provider-proxy/package.json (2)
25-35: LGTM on dependency updates.The
@akashnetwork/chain-sdkversion bump to1.0.0-alpha.18and the addition ofcockatielfor retry logic align with the PR objective to migrate to gRPC-based Chain SDK calls.
29-29: The review comment's premise appears to be incorrect.Based on web search results,
@cosmjs/encodingis a standalone utility package providing encoding/decoding helpers (bech32, hex, base64, utf8) essential for CosmJS-based applications. The claim that it's redundant because "chain-sdk provides all cosmjs types" is not accurate—chain-sdkis a separate, independent Node SDK unrelated to the CosmJS ecosystem, and@cosmjs/encodingprovides utility functions rather than just types. This dependency should likely remain.apps/provider-proxy/test/setup/proxyServer.ts (1)
7-10: LGTM!Using
Partial<AppConfig>provides appropriate flexibility for test setup, allowing tests to specify only the configuration properties they need while still enforcingPORT: 0for dynamic port allocation.apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (3)
123-133: Setup function follows coding guidelines.The
setupfunction is correctly placed at the bottom of thedescribeblock, accepts a single parameter with inline type definition, and returns the object under test. This aligns with the coding guidelines for test files.
11-29: Test structure and assertions are well-designed.The test correctly verifies:
- Query parameters structure including the hex-to-decimal serial number conversion
- The expected
nullreturn value when no certificates are found- Proper mocking of the
getCertificatesfunction
64-71: Based on the cockatiel documentation, I've found the key detail:
maxAttemptsis the total number of attempts (including the initial try). Retries performed = maxAttempts − 1.This means:
maxAttempts: 3= 1 initial attempt + 2 retries = 3 total function calls- The test expects 4 calls at line 70
This indicates a mismatch between the test expectation and what
maxAttempts: 3should produce.However, I cannot access the repository to verify:
- Whether ProviderService actually uses
maxAttempts: 3or something else- Whether the past review's "addressed in commits" actually fixed the implementation or test
- If there are other retry mechanisms involved
Retry count expectation at line 70 appears inconsistent with cockatiel's
maxAttemptssemantics. According to cockatiel documentation,maxAttempts: 3should result in exactly 3 total function calls (1 initial + 2 retries), not 4. Verify whether ProviderService usesmaxAttempts: 3ormaxAttempts: 4, and ensure the test expectation matches the actual implementation.apps/provider-proxy/src/services/ProviderService/ProviderService.ts (2)
22-31: LGTM on retry policy implementation.The retry policy using cockatiel with
ExponentialBackoffand conditional handling based on error codes is well-structured.
38-62: Certificate fetching logic handles edge cases appropriately.The method correctly:
- Builds query parameters with proper hex-to-decimal serial number conversion
- Handles single certificate response defensively with optional chaining
- Logs errors with relevant context (providerAddress, serialNumber)
- Returns
nullfor error cases to prevent cascading failuresapps/provider-proxy/env/.env.mainnet (1)
1-1: Endpoint verified as valid and operational.The gRPC endpoint
https://akash.lavenderfive.com:443is confirmed as a legitimate Akash mainnet gRPC endpoint maintained by Lavender.Five. The configuration change aligns with the PR objective to use gRPC instead of REST. Port 443 is the correct standard for gRPC-web over HTTPS. No issues identified.apps/provider-proxy/env/.env.sandbox (1)
1-1: LGTM!Environment variable correctly renamed to reflect the gRPC endpoint usage.
apps/provider-proxy/src/config/env.config.ts (1)
3-6: LGTM!Schema correctly updated to use
GRPC_NODE_URLwith appropriate URL validation.apps/provider-proxy/test/functional/provider-proxy-http.spec.ts (4)
17-19: Proper async cleanup pattern.Good use of
Promise.allto parallelize server shutdowns and ensure all cleanup operations are awaited.
72-78: Test title and setup correctly updated for gRPC flow.Test accurately describes caching behavior when gRPC is unavailable, and the setup uses the new
GRPC_NODE_URLconfiguration.
225-256: Retry test correctly validates gRPC resilience.Test properly verifies the retry logic when the gRPC server is initially unavailable, then comes back online.
258-291: gRPC error 14/unavailable retry test is well structured.The interceptor pattern correctly simulates a transient gRPC error (status 14) followed by recovery.
apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts (2)
12-15: Proper async cleanup for WebSocket tests.The
afterEachcorrectly usesPromise.allto cleanly shut down all servers, preventing resource leaks between tests.
37-38: Configuration correctly updated throughout WebSocket tests.All
startServercalls consistently useGRPC_NODE_URLwith the chain server URL.apps/provider-proxy/src/container.ts (2)
1-1: LGTM!Import of
createChainNodeSDKaligns with the PR objective to integrate chain-sdk.
17-24: Chain SDK configuration is correctly implemented with the documentedquery.baseUrlpattern.The
createChainNodeSDKAPI accepts the configuration object structure used in the code. Thequery.baseUrlsetting for read-only operations is the standard documented pattern for the @akashnetwork/chain-sdk library, and the implementation inProviderServiceinstantiation correctly omits thetxconfiguration, which aligns with read-only usage.apps/provider-proxy/test/setup/chainApiServer.ts (8)
57-62: Good use of protobuf decoding from chain-sdk.The implementation correctly uses
QueryCertificatesRequest.decode()to parse the gRPC payload after stripping the 5-byte frame header, addressing the previous fragile regex parsing concern.
100-118: Implementation looks correct.The early return when the server isn't listening is good defensive coding. The optional chaining on
grpcServerhandles the undefined case properly.
120-123: Clean interface definition.The simplified
interceptRequestsignature that returns a boolean for short-circuiting is appropriate for this test helper.
125-133: LGTM for test purposes.Using
Math.random()for test address generation is acceptable. For production code,crypto.getRandomValues()would be preferred, but this is fine for test infrastructure.
135-154: Good implementation using chain-sdk types.The helper correctly converts
X509Certificateto the protobuf format expected byQueryCertificatesResponse, using theState.validenum from chain-sdk as suggested in past reviews.
156-167: Proper use of chain-sdk encoding.The implementation correctly uses
QueryCertificatesResponse.fromPartial()andencode().finish()as suggested in past reviews, ensuring type-safe protobuf encoding.
169-181: Correct gRPC framing implementation.The 5-byte frame header (compression flag + big-endian length) and the response headers (
grpc-status: "0") follow the gRPC over HTTP/2 specification correctly.
67-68: Based on my web search verification,toLegacyObject()is NOT deprecated in Node.js. The method remains fully documented and available in the Node.js crypto API. While modern alternatives liketoJSON()and direct property access (publicKey,validFromDate) exist, they are optional enhancements, not required replacements.The code at lines 67-68 is using
toLegacyObject()appropriately and does not require changes based on deprecation concerns.
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
9-12: Consider removing error code 13 (INTERNAL) from retriable codes.Per gRPC best practices, INTERNAL (code 13) represents serious invariant/binary errors that typically indicate bugs rather than transient failures. UNAVAILABLE (code 14) is the canonical transient code for retries. Automatically retrying INTERNAL errors may mask server-side bugs.
const RETRIABLE_ERROR_CODES = [ - 13, // internal 14 // unavailable ];
🧹 Nitpick comments (2)
apps/provider-proxy/test/setup/chainApiServer.ts (1)
98-114: Reset module state after server close for test isolation.After closing the server,
grpcServershould be reset toundefinedto ensure clean state for subsequent test runs that may callstartChainApiServeragain.grpcServer?.close(error => { if (error) { reject(error); } else { + grpcServer = undefined; resolve(); } });apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
23-31: The non-null assertion onerror.code!is potentially fragile.The
hasErrorCodetype guard only confirms that"code" in erroris true, butcodecould still beundefined. WhileRETRIABLE_ERROR_CODES.includes(undefined)returnsfalse(safe behavior), the non-null assertion is misleading.private retryPolicy = retry( handleWhen(error => { - return hasErrorCode(error) && RETRIABLE_ERROR_CODES.includes(error.code!); + return hasErrorCode(error) && typeof error.code === "number" && RETRIABLE_ERROR_CODES.includes(error.code); }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts(1 hunks)apps/provider-proxy/test/setup/chainApiServer.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/provider-proxy/test/setup/chainApiServer.tsapps/provider-proxy/src/services/ProviderService/ProviderService.ts
🧠 Learnings (4)
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/provider-proxy/test/setup/chainApiServer.ts
📚 Learning: 2025-10-31T11:26:42.138Z
Learnt from: stalniy
Repo: akash-network/console PR: 2138
File: apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts:379-382
Timestamp: 2025-10-31T11:26:42.138Z
Learning: In TypeScript/JavaScript, the pattern of checking a cached value and then performing an async operation to fetch it without proper synchronization is race condition unsafe:
```typescript
private async getAddress() {
if (!this.address) {
this.address = await this.wallet.getFirstAddress();
}
return this.address;
}
```
Multiple concurrent calls can all pass the `if (!this.address)` check before any of them sets the value, leading to duplicate async operations. This should be flagged as a race condition. Proper synchronization (mutex, atomic promise caching, or guaranteed single-threaded execution) is required.
Applied to files:
apps/provider-proxy/test/setup/chainApiServer.ts
📚 Learning: 2025-07-03T14:40:49.886Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use type `any` or cast to type `any`. Always define the proper TypeScript types.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
🧬 Code graph analysis (1)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
packages/logging/src/services/logger/logger.service.ts (1)
error(141-143)
⏰ 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: 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: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (10)
apps/provider-proxy/test/setup/chainApiServer.ts (6)
1-8: LGTM! Clean imports and proper use of chain-sdk types.The imports correctly use
QueryCertificatesRequest,QueryCertificatesResponse, andStatefrom the chain-sdk private types, addressing the previous review feedback to use proper protobuf encoding/decoding instead of fragile regex parsing.
10-96: Well-structured gRPC mock server implementation.The stream-based handling with proper protobuf decoding via
QueryCertificatesRequest.decode()is clean. The 5-byte frame stripping at line 58 correctly handles gRPC message framing.One minor observation: the
server.listencallback doesn't handle potential errors from the listen operation itself, but this is acceptable for test infrastructure.
116-119: LGTM!The interface is clean and well-typed. The
interceptRequestcallback signature is simple and appropriate for test interception.
121-129: LGTM!Using
Math.random()for generating test addresses is acceptable. For production use,crypto.getRandomValues()would be preferred, but this is test infrastructure.
131-136: LGTM!Clean session cleanup implementation using
pop()to drain the array while destroying each session.
138-184: Excellent use of chain-sdk types for protobuf encoding.The implementation correctly uses
QueryCertificatesResponse.encode()andfromPartial()as suggested in past reviews. The gRPC frame construction (compression flag byte + 4-byte big-endian length) at lines 173-176 follows the gRPC wire protocol correctly.apps/provider-proxy/src/services/ProviderService/ProviderService.ts (4)
1-7: LGTM!Clean type imports and the
ChainNodeSDKtype alias usingReturnType<typeof createChainNodeSDK>is a good pattern for deriving types from factory functions.
14-20: LGTM!Clean type guard implementation for narrowing
unknownerrors to check for thecodeproperty. This follows TypeScript best practices for safe error handling.
38-62: LGTM! Good defensive error handling.The defensive certificate access pattern at lines 52-54 properly addresses the past review comment about null safety. The error handling with
unknowntype and logging follows best practices while maintaining process resilience by returningnullrather than throwing.
64-75: LGTM!Clean implementation of
fetchCertificatewith retry policy delegation. TheisValidationServerErrormethod correctly identifies validation-related error messages.
jzsfkzm
force-pushed
the
features/2188-certificates-from-grpc
branch
from
38e2081 to
b64eaa1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (1)
64-71: Fix the retry count expectation.The test expects 4 calls, but the ProviderService is configured with
maxAttempts: 3. In cockatiel,maxAttemptsrepresents the total number of attempts (1 initial + 2 retries = 3 total), not the number of retries. This is inconsistent with line 61, which correctly expects 3 calls.Apply this diff:
- expect(getCertificates).toHaveBeenCalledTimes(4); + expect(getCertificates).toHaveBeenCalledTimes(3);Additionally, consider updating the test description for clarity:
- it("retries getCertificates() request 3 times and then fails in case of grpc-status=14", async () => { + it("attempts getCertificates() request 3 times (1 initial + 2 retries) and then fails in case of grpc-status=14", async () => {apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
9-12: Remove error code 13 (INTERNAL) from retry logic.Error code 13 (gRPC INTERNAL) represents server-side implementation errors that should not be automatically retried. This was previously flagged as a blocking issue. Additionally, consider adding error code 4 (DEADLINE_EXCEEDED) to handle timeout scenarios.
Apply this diff:
const RETRIABLE_ERROR_CODES = [ - 13, // internal + 4, // deadline_exceeded - timeout scenarios 14 // unavailable ];Based on past review discussions, error code 14 (UNAVAILABLE) covers transient network failures, service downtime, and redeployment scenarios. Error code 4 handles cases where the service is unresponsive or extremely slow without breaking the connection.
🧹 Nitpick comments (2)
apps/provider-proxy/test/setup/chainApiServer.ts (1)
57-62: Consider documenting the gRPC framing protocol.The 5-byte slice at line 58 implements standard gRPC framing (1-byte compression flag + 4-byte message length), but this isn't immediately obvious to future maintainers.
Consider adding a comment:
if (requestData.length > 5) { + // gRPC framing: skip 5-byte header (1 byte compression flag + 4 bytes message length) const protoData = requestData.slice(5); const certRequest = QueryCertificatesRequest.decode(protoData);apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
14-26: Consider avoiding non-null assertion in retry policy.While the non-null assertion
error.code!at line 25 is technically safe (becausehasErrorCodeconfirms the property exists), it could be more explicit for maintainability.Consider refining the type guard and interface:
interface ErrorWithCode { - code?: number; + code: number; } function hasErrorCode(error: unknown): error is ErrorWithCode { - return typeof error === "object" && error !== null && "code" in error; + return typeof error === "object" && error !== null && "code" in error && typeof (error as any).code === "number"; }Then remove the non-null assertion:
- return hasErrorCode(error) && RETRIABLE_ERROR_CODES.includes(error.code!); + return hasErrorCode(error) && RETRIABLE_ERROR_CODES.includes(error.code);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
apps/provider-proxy/env/.env.mainnet(1 hunks)apps/provider-proxy/env/.env.sample(1 hunks)apps/provider-proxy/env/.env.sandbox(1 hunks)apps/provider-proxy/package.json(2 hunks)apps/provider-proxy/src/config/env.config.ts(1 hunks)apps/provider-proxy/src/container.ts(2 hunks)apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts(2 hunks)apps/provider-proxy/src/services/ProviderService/ProviderService.ts(1 hunks)apps/provider-proxy/test/functional/provider-proxy-http.spec.ts(26 hunks)apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts(10 hunks)apps/provider-proxy/test/setup/chainApiServer.ts(1 hunks)apps/provider-proxy/test/setup/proxyServer.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/provider-proxy/package.json
- apps/provider-proxy/env/.env.mainnet
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.tsapps/provider-proxy/test/functional/provider-proxy-ws.spec.tsapps/provider-proxy/test/functional/provider-proxy-http.spec.tsapps/provider-proxy/test/setup/proxyServer.tsapps/provider-proxy/src/services/ProviderService/ProviderService.tsapps/provider-proxy/test/setup/chainApiServer.tsapps/provider-proxy/src/container.tsapps/provider-proxy/src/config/env.config.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.tsapps/provider-proxy/test/functional/provider-proxy-ws.spec.tsapps/provider-proxy/test/functional/provider-proxy-http.spec.ts
🧠 Learnings (9)
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.tsapps/provider-proxy/test/functional/provider-proxy-ws.spec.tsapps/provider-proxy/test/functional/provider-proxy-http.spec.tsapps/provider-proxy/src/services/ProviderService/ProviderService.tsapps/provider-proxy/test/setup/chainApiServer.ts
📚 Learning: 2025-06-08T03:07:13.871Z
Learnt from: stalniy
Repo: akash-network/console PR: 1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts
📚 Learning: 2025-11-25T17:45:58.258Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/test-descriptions.mdc:0-0
Timestamp: 2025-11-25T17:45:58.258Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `<Subject>.name` in the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Applied to files:
apps/provider-proxy/test/functional/provider-proxy-ws.spec.tsapps/provider-proxy/test/functional/provider-proxy-http.spec.ts
📚 Learning: 2025-11-25T17:45:49.180Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-11-25T17:45:49.180Z
Learning: Applies to {apps/deploy-web,apps/provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations
Applied to files:
apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts
📚 Learning: 2025-09-25T20:34:55.117Z
Learnt from: jigar-arc10
Repo: akash-network/console PR: 1970
File: apps/provider-console/src/config/network.config.ts:17-28
Timestamp: 2025-09-25T20:34:55.117Z
Learning: In cosmos-kit integration, nodesUrl and versionUrl should point to Cosmos API endpoints (not console backend API endpoints) as cosmos-kit expects these to be Cosmos REST endpoints for proper functionality.
Applied to files:
apps/provider-proxy/env/.env.sandboxapps/provider-proxy/env/.env.sample
📚 Learning: 2025-07-03T14:40:49.886Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use type `any` or cast to type `any`. Always define the proper TypeScript types.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
📚 Learning: 2025-10-31T11:26:42.138Z
Learnt from: stalniy
Repo: akash-network/console PR: 2138
File: apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts:379-382
Timestamp: 2025-10-31T11:26:42.138Z
Learning: In TypeScript/JavaScript, the pattern of checking a cached value and then performing an async operation to fetch it without proper synchronization is race condition unsafe:
```typescript
private async getAddress() {
if (!this.address) {
this.address = await this.wallet.getFirstAddress();
}
return this.address;
}
```
Multiple concurrent calls can all pass the `if (!this.address)` check before any of them sets the value, leading to duplicate async operations. This should be flagged as a race condition. Proper synchronization (mutex, atomic promise caching, or guaranteed single-threaded execution) is required.
Applied to files:
apps/provider-proxy/test/setup/chainApiServer.ts
📚 Learning: 2025-11-12T09:03:40.132Z
Learnt from: stalniy
Repo: akash-network/console PR: 0
File: :0-0
Timestamp: 2025-11-12T09:03:40.132Z
Learning: For backend services (like the Indexer), prefer using createOtelLogger from "akashnetwork/logging/otel" to include OpenTelemetry trace context in logs.
Applied to files:
apps/provider-proxy/src/container.ts
🧬 Code graph analysis (6)
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (3)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
ProviderService(22-75)packages/http-sdk/src/certificates/certificates.service.ts (1)
getCertificates(25-37)apps/provider-proxy/test/seeders/createX509CertPair.ts (2)
CertificateOptions(33-38)createX509CertPair(4-26)
apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts (3)
apps/provider-proxy/test/setup/proxyServer.ts (2)
stopServer(12-14)startServer(7-10)apps/provider-proxy/test/setup/providerServer.ts (1)
stopProviderServer(73-75)apps/provider-proxy/test/setup/chainApiServer.ts (1)
stopChainApiServer(98-114)
apps/provider-proxy/test/functional/provider-proxy-http.spec.ts (2)
apps/provider-proxy/test/setup/proxyServer.ts (2)
stopServer(12-14)startServer(7-10)apps/provider-proxy/test/setup/chainApiServer.ts (3)
stopChainApiServer(98-114)generateBech32(121-129)startChainApiServer(10-96)
apps/provider-proxy/test/setup/proxyServer.ts (1)
apps/provider-proxy/src/config/env.config.ts (1)
AppConfig(8-8)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
packages/logging/src/services/logger/logger.service.ts (1)
error(141-143)
apps/provider-proxy/src/container.ts (1)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
ProviderService(22-75)
⏰ 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: 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: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
🔇 Additional comments (14)
apps/provider-proxy/env/.env.sample (1)
1-1: LGTM!The environment variable update from REST_API_NODE_URL to GRPC_NODE_URL with the gRPC endpoint is correct and aligns with the PR objective to migrate from REST-based to gRPC-based certificate fetching.
apps/provider-proxy/env/.env.sandbox (1)
1-1: LGTM!Consistent update matching the .env.sample file.
apps/provider-proxy/test/setup/proxyServer.ts (1)
7-7: LGTM!Changing the parameter type to
Partial<AppConfig>improves test flexibility by allowing tests to provide only the necessary configuration fields rather than requiring the complete config object.apps/provider-proxy/src/config/env.config.ts (1)
4-4: LGTM!The schema update correctly replaces REST_API_NODE_URL with GRPC_NODE_URL while maintaining proper URL validation. This is the core configuration change enabling the gRPC migration.
apps/provider-proxy/test/functional/provider-proxy-http.spec.ts (1)
8-8: LGTM!The test file has been consistently updated to use the gRPC-based flow. Variable names, server initialization, and test descriptions properly reflect the migration from REST API to gRPC endpoints. The retry logic tests correctly cover gRPC-specific error scenarios (grpc-status=14).
Also applies to: 18-18, 25-27, 50-52, 76-78, 111-129, 152-157, 188-192, 225-256, 258-291, 316-317, 361-362, 395-396, 419-420, 453-454, 485-486, 524-545, 574-591, 618-630, 688-699
apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts (1)
8-8: LGTM!The WebSocket test file has been consistently updated to use the gRPC-based flow. The concurrent shutdown using
Promise.allin theafterEachhook is a good practice that improves test cleanup efficiency.Also applies to: 13-14, 37-37, 68-68, 88-88, 116-116, 139-139, 167-167, 194-194, 242-242, 305-305
apps/provider-proxy/src/container.ts (1)
1-1: LGTM!The ProviderService instantiation correctly migrates from REST-based fetch to the ChainNodeSDK client using gRPC. The SDK is properly configured with the GRPC_NODE_URL and, as per the PR objectives, does not include the
txoption, making it read-only for certificate queries.Also applies to: 17-24
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (1)
1-7: Test structure looks good.The test migration to use ChainNodeSDK-based mocking is well-structured. The setup function correctly follows coding guidelines by being at the bottom and accepting inline parameter types. The use of
ProviderService.namein the describe block is also correct.Also applies to: 11-80, 123-137
apps/provider-proxy/test/setup/chainApiServer.ts (3)
1-8: LGTM! Proper use of chain-sdk protobuf types.The imports and server setup correctly use the generated protobuf types from
@akashnetwork/chain-sdk, and session tracking is properly initialized.
64-75: LGTM! Correct certificate filtering logic.The certificate matching logic properly handles optional owner and serial filters, and the hex-to-decimal serial number conversion is correct.
159-184: LGTM! Proper protobuf encoding and gRPC response framing.The response construction correctly uses
QueryCertificatesResponse.encodewithfromPartial, and thesendResponsefunction properly implements gRPC framing (5-byte header + protobuf payload) with appropriate headers.apps/provider-proxy/src/services/ProviderService/ProviderService.ts (3)
1-7: LGTM! Clean imports and type definitions.The imports correctly use types from
@akashnetwork/chain-sdk, and theChainNodeSDKtype alias properly captures the return type ofcreateChainNodeSDK.
38-62: LGTM! Proper error handling and type safety.The method correctly constructs the query parameters using
QueryInput, handles errors withunknowntype (addressing past review feedback), and uses optional chaining to safely extract the certificate.
64-68: LGTM! Clean retry policy integration.The private
fetchCertificatemethod properly integrates the retry policy with the SDK client call, maintaining clean separation of concerns.
|
@jzsfkzm all tests currently pass for console-api in other PRs, so this failure relates to changes in provider-proxy. There are some tests that rely on provider-proxy behavior (they are almost e2e and this is not good for PR level test). I assume that sandbox blockchain exposes grpc server over plaintext, not SSL |
|
Yes, I'm right: |
jzsfkzm
force-pushed
the
features/2188-certificates-from-grpc
branch
2 times, most recently
from
a484d13 to
f71e1ed
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (1)
64-71: Fix the retry count expectation.The test expects 4 calls, but
ProviderServiceis configured withmaxAttempts: 3(line 28 in ProviderService.ts). In cockatiel,maxAttemptsrepresents the total number of attempts including the initial one, so this should be 3 total calls, not 4.Apply this diff to fix the expectation:
- expect(getCertificates).toHaveBeenCalledTimes(4); + expect(getCertificates).toHaveBeenCalledTimes(3);Note: This was flagged in a previous review and marked as addressed, but the code still expects 4 calls.
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
9-12: Remove error code 13 from RETRIABLE_ERROR_CODES.gRPC error code 13 (INTERNAL) represents serious server-side errors and should not be automatically retried. The gRPC best practice is to only retry UNAVAILABLE (code 14) for transient network failures. This was flagged in a previous review as a blocking issue.
Apply this diff:
const RETRIABLE_ERROR_CODES = [ - 13, // internal 14 // unavailable ];As per past review feedback.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
apps/provider-proxy/env/.env.mainnet(1 hunks)apps/provider-proxy/env/.env.sample(1 hunks)apps/provider-proxy/env/.env.sandbox(1 hunks)apps/provider-proxy/package.json(2 hunks)apps/provider-proxy/src/config/env.config.ts(1 hunks)apps/provider-proxy/src/container.ts(2 hunks)apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts(2 hunks)apps/provider-proxy/src/services/ProviderService/ProviderService.ts(1 hunks)apps/provider-proxy/test/functional/provider-proxy-http.spec.ts(26 hunks)apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts(10 hunks)apps/provider-proxy/test/setup/chainApiServer.ts(1 hunks)apps/provider-proxy/test/setup/proxyServer.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/provider-proxy/env/.env.mainnet
- apps/provider-proxy/env/.env.sandbox
- apps/provider-proxy/test/setup/proxyServer.ts
- apps/provider-proxy/src/container.ts
- apps/provider-proxy/env/.env.sample
- apps/provider-proxy/src/config/env.config.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/provider-proxy/test/functional/provider-proxy-ws.spec.tsapps/provider-proxy/src/services/ProviderService/ProviderService.spec.tsapps/provider-proxy/test/setup/chainApiServer.tsapps/provider-proxy/test/functional/provider-proxy-http.spec.tsapps/provider-proxy/src/services/ProviderService/ProviderService.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/provider-proxy/test/functional/provider-proxy-ws.spec.tsapps/provider-proxy/src/services/ProviderService/ProviderService.spec.tsapps/provider-proxy/test/functional/provider-proxy-http.spec.ts
🧠 Learnings (9)
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/provider-proxy/test/functional/provider-proxy-ws.spec.tsapps/provider-proxy/src/services/ProviderService/ProviderService.spec.tsapps/provider-proxy/package.jsonapps/provider-proxy/test/setup/chainApiServer.tsapps/provider-proxy/test/functional/provider-proxy-http.spec.tsapps/provider-proxy/src/services/ProviderService/ProviderService.ts
📚 Learning: 2025-11-25T17:45:49.180Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-11-25T17:45:49.180Z
Learning: Applies to {apps/deploy-web,apps/provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations
Applied to files:
apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts
📚 Learning: 2025-11-25T17:45:58.258Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/test-descriptions.mdc:0-0
Timestamp: 2025-11-25T17:45:58.258Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `<Subject>.name` in the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Applied to files:
apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts
📚 Learning: 2025-11-25T17:45:52.965Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-11-25T17:45:52.965Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files. The `setup` function must be at the bottom of the root `describe` block, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
Applied to files:
apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts
📚 Learning: 2025-06-08T03:07:13.871Z
Learnt from: stalniy
Repo: akash-network/console PR: 1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts
📚 Learning: 2025-09-04T04:27:50.638Z
Learnt from: stalniy
Repo: akash-network/console PR: 1868
File: apps/api/src/billing/services/managed-signer/managed-signer.service.ts:1-1
Timestamp: 2025-09-04T04:27:50.638Z
Learning: In the akash-network/console project, importing MsgCreateLease from "akashnetwork/akash-api/v1beta3" instead of v1beta4 is considered non-critical by the maintainers, likely due to backward compatibility or project-specific requirements.
Applied to files:
apps/provider-proxy/package.json
📚 Learning: 2025-10-31T11:26:42.138Z
Learnt from: stalniy
Repo: akash-network/console PR: 2138
File: apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts:379-382
Timestamp: 2025-10-31T11:26:42.138Z
Learning: In TypeScript/JavaScript, the pattern of checking a cached value and then performing an async operation to fetch it without proper synchronization is race condition unsafe:
```typescript
private async getAddress() {
if (!this.address) {
this.address = await this.wallet.getFirstAddress();
}
return this.address;
}
```
Multiple concurrent calls can all pass the `if (!this.address)` check before any of them sets the value, leading to duplicate async operations. This should be flagged as a race condition. Proper synchronization (mutex, atomic promise caching, or guaranteed single-threaded execution) is required.
Applied to files:
apps/provider-proxy/test/setup/chainApiServer.ts
📚 Learning: 2025-07-03T14:40:49.886Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use type `any` or cast to type `any`. Always define the proper TypeScript types.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
🧬 Code graph analysis (2)
apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts (3)
apps/provider-proxy/test/setup/proxyServer.ts (2)
stopServer(12-14)startServer(7-10)apps/provider-proxy/test/setup/providerServer.ts (1)
stopProviderServer(73-75)apps/provider-proxy/test/setup/chainApiServer.ts (1)
stopChainApiServer(98-114)
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (3)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
ProviderService(22-75)packages/http-sdk/src/certificates/certificates.service.ts (1)
getCertificates(25-37)apps/provider-proxy/test/seeders/createX509CertPair.ts (2)
CertificateOptions(33-38)createX509CertPair(4-26)
⏰ 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/provider-console) / validate-unsafe
- GitHub Check: validate (apps/deploy-web) / validate-unsafe
- 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 / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
🔇 Additional comments (8)
apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts (1)
8-8: LGTM—Test setup updated for gRPC flow.The renamed function
stopChainApiServer(consistent JavaScript casing) and concurrent teardown usingPromise.allproperly reflect the migration to gRPC-based certificate retrieval.Also applies to: 13-15
apps/provider-proxy/test/functional/provider-proxy-http.spec.ts (1)
8-8: LGTM—HTTP proxy tests updated for gRPC.The tests have been properly updated to use the gRPC-based chain API server with:
- Renamed
stopChainApiServerfor consistent casing- Concurrent teardown in
afterEach- Updated variable naming (
grpcServerinstead ofchainServer)- Simplified
interceptRequestsignature to a parameterless function returning booleanAlso applies to: 18-18, 25-27, 267-274
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (2)
1-7: LGTM—Test setup migrated to ChainNodeSDK.The test properly imports
createChainNodeSDKandSDKErrorfrom chain-sdk, defining a type alias for the return type. This aligns with the production implementation.
123-137: LGTM—Test setup function follows guidelines.The
setupfunction is correctly placed at the bottom of the describe block, accepts inline-typed parameters, creates and returns the service under test, and avoids shared state.apps/provider-proxy/test/setup/chainApiServer.ts (2)
1-1: LGTM—Proper protobuf handling implemented.The test server correctly uses protobuf decode/encode from
@akashnetwork/chain-sdk/private-types/akash.v1instead of fragile regex parsing. This addresses the previous review feedback and provides robust request/response handling for the gRPC-like test server.Based on past review feedback.
Also applies to: 57-62, 159-170
7-8: LGTM—Proper session lifecycle management.The HTTP/2 server implementation correctly tracks sessions in an array and provides
destroyAllSessionsfor cleanup. This prevents resource leaks during test teardown.Also applies to: 20-23, 131-136
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (2)
23-31: LGTM—Retry policy properly configured.The retry policy uses cockatiel's
ExponentialBackoffwithmaxAttempts: 3and correctly handles retriable errors. ThefetchCertificatemethod properly wraps the SDK call with retry logic.Also applies to: 64-68
33-36: LGTM—ChainNodeSDK integration and error handling.The constructor properly accepts a
ChainNodeSDKclient, andgetCertificatecorrectly:
- Builds query parameters using
QueryInput<QueryCertificatesRequest>- Handles the single-certificate case with safe optional chaining
- Catches and logs errors with proper context (providerAddress, serialNumber)
- Uses
unknowntype for error handling per coding guidelinesAlso applies to: 38-62
stalniy
force-pushed
the
features/2188-certificates-from-grpc
branch
from
f71e1ed to
35388a9
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (1)
64-71: Fix retry call-count expectation for grpc-status=14 failuresIn the
"retries getCertificates() request 3 times and then fails in case of grpc-status=14"test, you currently expect:expect(getCertificates).toHaveBeenCalledTimes(4);But
ProviderServiceconfigures cockatiel withmaxAttempts: 3, which is the total number of attempts (1 initial + 2 retries). When all attempts fail with code 14,getCertificateswill be called exactly 3 times.Update the expectation to:
expect(getCertificates).toHaveBeenCalledTimes(3);so the test matches the actual retry policy.
#!/bin/bash # Verify maxAttempts configuration and ensure tests match it echo "ProviderService retry policy:" rg -n "maxAttempts" apps/provider-proxy/src/services/ProviderService/ProviderService.ts echo echo "Check the failing expectation in the spec:" rg -n "retries getCertificates\\(\\) request 3 times" apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts -n -C3apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
9-31: Reconsider retrying on gRPC INTERNAL (code 13)
RETRIABLE_ERROR_CODEScurrently includes both 13 (INTERNAL) and 14 (UNAVAILABLE):const RETRIABLE_ERROR_CODES = [ 13, // internal 14 // unavailable ];Automatically retrying INTERNAL errors can mask real server bugs or invariant violations; UNAVAILABLE (14) is the canonical transient/network code, and prior review already suggested restricting retries to 14 only. Consider:
- Dropping 13 from the retriable list so only clearly transient 14 is retried.
- Optionally tightening
hasErrorCodeto require a numericcode:function hasErrorCode(error: unknown): error is ErrorWithCode { return ( typeof error === "object" && error !== null && typeof (error as ErrorWithCode).code === "number" ); }This keeps retries focused on truly transient failures and avoids surprising behavior on other error shapes.
In cockatiel-based retry logic for a gRPC client, should gRPC status code 13 (INTERNAL) typically be treated as retriable, or is it safer to only retry code 14 (UNAVAILABLE)? Provide guidance from gRPC docs and common best practices.
🧹 Nitpick comments (2)
apps/provider-proxy/test/setup/chainApiServer.ts (1)
1-184: HTTP/2 gRPC test server logic looks solidThe http2-based mock gRPC server correctly:
- Tracks sessions and destroys them on shutdown.
- Validates the
:pathand returns 404 for non‑Certificates calls.- Parses the gRPC frame (compression flag + length prefix) before decoding
QueryCertificatesRequest.- Filters certificates by owner/serial and encodes a
QueryCertificatesResponse, framing it properly for gRPC.As an optional clean‑up, you could reset
sessions.length = 0insidestartChainApiServer(or keep sessions per server) to make the lifecycle more explicit, but it’s not functionally required for tests.apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts (1)
123-137: Test setup helper matches DI pattern; minor placement nit onlyThe
setuphelper cleanly injects a partialChainNodeSDKwith a mockedgetCertificates, which is exactly what you want for isolatingProviderService. As a minor style alignment with the testing guidelines, you could movebuildCertificateabovesetupso thatsetupis the last function in the rootdescribeblock, but that’s purely cosmetic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
apps/provider-proxy/env/.env.mainnet(1 hunks)apps/provider-proxy/env/.env.sample(1 hunks)apps/provider-proxy/env/.env.sandbox(1 hunks)apps/provider-proxy/package.json(2 hunks)apps/provider-proxy/src/config/env.config.ts(1 hunks)apps/provider-proxy/src/container.ts(2 hunks)apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts(2 hunks)apps/provider-proxy/src/services/ProviderService/ProviderService.ts(1 hunks)apps/provider-proxy/test/functional/provider-proxy-http.spec.ts(26 hunks)apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts(10 hunks)apps/provider-proxy/test/setup/chainApiServer.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/provider-proxy/env/.env.sample
- apps/provider-proxy/package.json
- apps/provider-proxy/env/.env.sandbox
- apps/provider-proxy/src/config/env.config.ts
- apps/provider-proxy/test/functional/provider-proxy-ws.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/provider-proxy/src/container.tsapps/provider-proxy/src/services/ProviderService/ProviderService.spec.tsapps/provider-proxy/test/functional/provider-proxy-http.spec.tsapps/provider-proxy/src/services/ProviderService/ProviderService.tsapps/provider-proxy/test/setup/chainApiServer.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.tsapps/provider-proxy/test/functional/provider-proxy-http.spec.ts
🧠 Learnings (7)
📚 Learning: 2025-11-12T09:03:40.132Z
Learnt from: stalniy
Repo: akash-network/console PR: 0
File: :0-0
Timestamp: 2025-11-12T09:03:40.132Z
Learning: For backend services (like the Indexer), prefer using createOtelLogger from "akashnetwork/logging/otel" to include OpenTelemetry trace context in logs.
Applied to files:
apps/provider-proxy/src/container.ts
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.tsapps/provider-proxy/test/functional/provider-proxy-http.spec.tsapps/provider-proxy/src/services/ProviderService/ProviderService.tsapps/provider-proxy/test/setup/chainApiServer.ts
📚 Learning: 2025-06-08T03:07:13.871Z
Learnt from: stalniy
Repo: akash-network/console PR: 1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.spec.ts
📚 Learning: 2025-09-25T20:34:55.117Z
Learnt from: jigar-arc10
Repo: akash-network/console PR: 1970
File: apps/provider-console/src/config/network.config.ts:17-28
Timestamp: 2025-09-25T20:34:55.117Z
Learning: In cosmos-kit integration, nodesUrl and versionUrl should point to Cosmos API endpoints (not console backend API endpoints) as cosmos-kit expects these to be Cosmos REST endpoints for proper functionality.
Applied to files:
apps/provider-proxy/env/.env.mainnet
📚 Learning: 2025-07-03T14:40:49.886Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use type `any` or cast to type `any`. Always define the proper TypeScript types.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
📚 Learning: 2025-10-31T11:26:42.138Z
Learnt from: stalniy
Repo: akash-network/console PR: 2138
File: apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts:379-382
Timestamp: 2025-10-31T11:26:42.138Z
Learning: In TypeScript/JavaScript, the pattern of checking a cached value and then performing an async operation to fetch it without proper synchronization is race condition unsafe:
```typescript
private async getAddress() {
if (!this.address) {
this.address = await this.wallet.getFirstAddress();
}
return this.address;
}
```
Multiple concurrent calls can all pass the `if (!this.address)` check before any of them sets the value, leading to duplicate async operations. This should be flagged as a race condition. Proper synchronization (mutex, atomic promise caching, or guaranteed single-threaded execution) is required.
Applied to files:
apps/provider-proxy/test/setup/chainApiServer.ts
🧬 Code graph analysis (3)
apps/provider-proxy/test/functional/provider-proxy-http.spec.ts (4)
apps/provider-proxy/test/setup/proxyServer.ts (2)
stopServer(12-14)startServer(7-10)apps/provider-proxy/test/setup/providerServer.ts (2)
stopProviderServer(73-75)startProviderServer(14-71)apps/provider-proxy/test/setup/chainApiServer.ts (3)
stopChainApiServer(98-114)generateBech32(121-129)startChainApiServer(10-96)apps/provider-proxy/test/seeders/createX509CertPair.ts (1)
createX509CertPair(4-26)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
packages/logging/src/services/logger/logger.service.ts (1)
error(141-143)
apps/provider-proxy/test/setup/chainApiServer.ts (1)
packages/http-sdk/src/certificates/certificates.service.ts (1)
getCertificates(25-37)
⏰ 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). (14)
- GitHub Check: log-collector-ci
- 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: 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
🔇 Additional comments (4)
apps/provider-proxy/env/.env.mainnet (1)
1-1: GRPC_NODE_URL wiring looks correctThe mainnet GRPC_NODE_URL value is well‑formed and consistent with the container wiring that passes it into
createChainNodeSDK({ query: { baseUrl } }). No issues here.apps/provider-proxy/test/functional/provider-proxy-http.spec.ts (1)
8-771: Functional tests correctly exercise GRPC_NODE_URL and gRPC failure modesThe test wiring and scenarios look consistent with the new gRPC/chain‑sdk flow:
startServer({ GRPC_NODE_URL: grpcServer.url })matches the container’s expectation forGRPC_NODE_URL.afterEachteardown now reliably stops proxy, provider, and chain gRPC servers, withstopChainApiServersafely handling mid‑test manual stops.- The new retry/caching tests (
can work without gRPC by using cached certificates,retries fetching chain certificates if gRPC is unavailable, andretries if gRPC responds with error 14/unavailable) align with the retry policy and cached certificate behavior inProviderService/CertificateValidator.No functional or guideline issues spotted here.
apps/provider-proxy/src/container.ts (1)
1-24: ProviderService is correctly switched to ChainNodeSDK over GRPC_NODE_URL
ProviderServiceis now constructed withcreateChainNodeSDK({ query: { baseUrl: appConfig.GRPC_NODE_URL } }), with notxwiring, which matches the requirement to use the chain‑sdk client in read‑only mode over gRPC. The change is minimal, type‑safe, and integrates cleanly with existing logging and DI.apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
38-68: Certificate query + decoding flow is correct and type-safe
- Using
QueryInput<QueryCertificatesRequest>withfilter.state = "valid", owner, and decimal serial computed viaBigInt(\0x${serialNumber}`).toString(10)` matches how serials are exposed by the chain and the test gRPC server.fetchCertificatewrapsgetCertificatesin a cockatiel retry policy, keeping retry concerns encapsulated.- The response handling correctly:
- Requires exactly one certificate.
- Safely retrieves
certificate?.certand returnsnullif missing.- The catch block uses
error: unknownand logs a structured object includingproviderAddress,serialNumber, anderror, in line with the “no any” guideline and the logger’sunknownsignature.This core flow looks good and aligns with the new chain‑sdk‑based design.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
10-18: Refine retryable error detection—avoid retrying onInternaland tighten the guardTwo points here:
Retrying on
SDKErrorCode.Internal
INTERNAL errors usually indicate a server bug or non‑transient failure. Auto‑retrying them can mask issues and add load without much benefit, especially since this is a read path and you already have a dedicated retry policy for transient network problems. Consider limiting retries to clearly transient codes such asSDKErrorCode.Unavailable(and, if the SDK exposes it distinctly, a timeout/deadline code), and stop retryingInternal.Stronger runtime guard for
error.code
hasErrorCodecurrently only checks that"code" in error. It may classify arbitrary objects (including ones with string codes like Node’sECONNREFUSED) asErrorWithCode, even thoughRETRIABLE_ERROR_CODESis numeric. You can cheaply tighten this to only accept numeric codes and remove the non‑null assertion:-const RETRIABLE_ERROR_CODES = [SDKErrorCode.Internal, SDKErrorCode.Unavailable]; +const RETRIABLE_ERROR_CODES = [SDKErrorCode.Unavailable]; -interface ErrorWithCode { - code?: number; -} +interface ErrorWithCode { + code: number; +} -function hasErrorCode(error: unknown): error is ErrorWithCode { - return typeof error === "object" && error !== null && "code" in error; -} +function hasErrorCode(error: unknown): error is ErrorWithCode { + if (typeof error !== "object" || error === null || !("code" in error)) { + return false; + } + + const code = (error as { code?: unknown }).code; + return typeof code === "number"; +} - handleWhen(error => { - return hasErrorCode(error) && RETRIABLE_ERROR_CODES.includes(error.code!); - }), + handleWhen(error => hasErrorCode(error) && RETRIABLE_ERROR_CODES.includes(error.code)),This keeps retries focused on the numeric SDK codes you actually want, and it avoids relying on
!forerror.code.Also applies to: 21-29
🧹 Nitpick comments (2)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (2)
32-32: Validate/document serial number format used for BigInt conversionThe query construction looks good and the use of
QueryInput<QueryCertificatesRequest>gives you nice typing. One nuance is the serial conversion:serial: BigInt(`0x${serialNumber}`).toString(10)This assumes
serialNumberis a hex string without a0xprefix. If upstream ever passes a decimal value, or an already‑prefixed hex string,BigIntwill throw synchronously and the error will be surfaced as a generic fetch failure.Consider either:
- Explicitly documenting that
serialNumbermust be a bare hex string, or- Normalizing/validating here (e.g., stripping a leading
0x, or short‑circuiting with a logged validation error and returningnull) so input‑format issues are not conflated with network/SDK errors.Also applies to: 37-46
48-52: Minor defensive and style tweaks in fetch/parse pathFunctionally this flow looks correct—single certificate is converted to
X509Certificate, and errors are logged with useful context plus the full error object.A couple of low‑impact refinements you might consider:
- Defensive check on
certificatesarray
If the SDK ever returnscertificatesasundefinedinstead of[]on “no results”,response.certificates.lengthwould throw and be caught as a generic error. Using optional chaining is slightly more robust:- const response = await this.fetchCertificate(queryParams); - if (response.certificates.length === 1) { + const response = await this.fetchCertificate(queryParams); + if (response.certificates?.length === 1) { const cert = response.certificates[0]?.certificate?.cert; return cert ? new X509Certificate(cert) : null; }
- Simplify
fetchCertificate’s async/await usage
SinceretryPolicy.executealready returns a promise of the inner function’s result, you don’t need both theasyncwrapper and the innerawait:- private async fetchCertificate(getCertificatesParams: QueryInput<QueryCertificatesRequest>): Promise<QueryCertificatesResponse> { - return this.retryPolicy.execute(async () => { - return await this.chainSdkClient.akash.cert.v1.getCertificates(getCertificatesParams); - }); - } + private fetchCertificate(getCertificatesParams: QueryInput<QueryCertificatesRequest>): Promise<QueryCertificatesResponse> { + return this.retryPolicy.execute(() => + this.chainSdkClient.akash.cert.v1.getCertificates(getCertificatesParams) + ); + }Also, good call on typing the catch parameter as
unknownand logging the whole error object; this aligns with the project’s “noany” guideline and preserves stack traces.Also applies to: 55-58, 60-60, 62-65
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
📚 Learning: 2025-07-03T14:40:49.886Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use type `any` or cast to type `any`. Always define the proper TypeScript types.
Applied to files:
apps/provider-proxy/src/services/ProviderService/ProviderService.ts
🧬 Code graph analysis (1)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
packages/logging/src/services/logger/logger.service.ts (1)
error(141-143)
⏰ 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). (11)
- GitHub Check: codecov/project/notifications
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
🔇 Additional comments (1)
apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
1-3: Imports and SDK typing look solidUsing
import typefor the SDK symbols and theChainNodeSDK = ReturnType<typeof createChainNodeSDK>alias keeps this service type‑safe while avoiding an unnecessary runtime dependency on the SDK factory in this module. No issues here.Also applies to: 5-5, 8-8
2 participants
closes #2188
Summary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.