feat: use JWT instead of certificates #1766
Conversation
WalkthroughReplaces certificate-based provider authentication with JWTs: adds a JwtTokenService and @akashnetwork/jwt, updates provider, deployment and lease flows to use walletId/JWT for manifest delivery and lease-status, removes certificate fields from APIs/examples/tests, and extends HTTP SDK for JWT-authenticated provider calls. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DeploymentWriter
participant ProviderService
participant JwtTokenService
participant Wallet
participant ProviderHTTP
Client->>DeploymentWriter: update(dseq, sdl, walletId)
DeploymentWriter->>ProviderService: sendManifest({walletId, dseq, manifest})
ProviderService->>JwtTokenService: generateJwtToken({walletId, leases: ["send-manifest"]})
JwtTokenService->>Wallet: instantiate wallet and create signer
JwtTokenService-->>ProviderService: jwtToken
ProviderService->>ProviderHTTP: sendManifest({hostUri, dseq, manifest, jwtToken})
ProviderHTTP->>Provider: PUT /deployment/{dseq}/manifest (Bearer JWT)
Provider-->>ProviderHTTP: 200
ProviderHTTP-->>ProviderService: response
ProviderService-->>DeploymentWriter: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~30 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
apps/api/package.json (1)
45-45: Consider pinning to a specific version for production stability.While the wildcard version may be acceptable for internal packages, consider using a specific version to ensure predictable builds and deployments.
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts (1)
19-19: Consider renaming for clarity.The
findWalletBymock function name could be more descriptive to indicate it's mocking thefindOneBymethod ofUserWalletRepository.Consider this naming improvement:
- const findWalletBy = jest.fn().mockImplementation(async () => newWallet); + const findOneBy = jest.fn().mockImplementation(async () => newWallet);And update the setup call accordingly:
- findWalletBy, + findOneBy,Also applies to: 26-26
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts (1)
82-89: Consider consistent mocking approach.The JWT module mocking mixes manual
jest.fn()with thejest-mock-extendedapproach used elsewhere. While functional, consider consistency.The current approach works well for this scenario since you're mocking static methods. The mix of approaches is acceptable here.
apps/api/src/provider/services/provider/provider.service.ts (1)
18-18: Address the dependency cycle detected by static analysis.Static analysis has identified a dependency cycle involving billing services, deployment services, and the JWT token service. This can lead to circular dependency issues and reduced maintainability.
Based on the retrieved learning about handling cross-cutting concerns, consider creating a separate GitHub issue to address this architectural concern rather than expanding the current PR scope.
The cycle path is:
@src/billing/services→managed-signer.service→trial-validation.service→deployment-reader.serviceConsider extracting shared concerns or restructuring the dependency graph to eliminate the cycle.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/api/package.json(1 hunks)apps/api/src/billing/lib/wallet/wallet.ts(1 hunks)apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts(3 hunks)apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts(3 hunks)apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts(2 hunks)apps/api/src/deployment/services/lease/lease.service.ts(1 hunks)apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts(1 hunks)apps/api/src/provider/services/jwt-token/jwt-token.service.ts(1 hunks)apps/api/src/provider/services/provider/provider.service.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/billing/lib/wallet/wallet.tsapps/api/src/deployment/services/lease/lease.service.tsapps/api/src/provider/services/jwt-token/jwt-token.service.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/deployment/services/deployment-writer/deployment-writer.service.tsapps/api/src/provider/services/jwt-token/jwt-token.service.spec.tsapps/api/src/provider/services/provider/provider.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/billing/lib/wallet/wallet.tsapps/api/src/deployment/services/lease/lease.service.tsapps/api/src/provider/services/jwt-token/jwt-token.service.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/deployment/services/deployment-writer/deployment-writer.service.tsapps/api/src/provider/services/jwt-token/jwt-token.service.spec.tsapps/api/src/provider/services/provider/provider.service.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: baktun14
PR: akash-network/console#1312
File: packages/jwt/src/jwt-token.ts:0-0
Timestamp: 2025-05-13T17:35:25.875Z
Learning: In the Akash JWT implementation, cryptographic signature verification is handled by the provider node code rather than in the JWT package itself. The JWT package only handles token creation, decoding, and basic validation (schema and time constraints).
Learnt from: baktun14
PR: akash-network/console#1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
📚 Learning: in the akash jwt implementation, cryptographic signature verification is handled by the provider nod...
Learnt from: baktun14
PR: akash-network/console#1312
File: packages/jwt/src/jwt-token.ts:0-0
Timestamp: 2025-05-13T17:35:25.875Z
Learning: In the Akash JWT implementation, cryptographic signature verification is handled by the provider node code rather than in the JWT package itself. The JWT package only handles token creation, decoding, and basic validation (schema and time constraints).
Applied to files:
apps/api/package.jsonapps/api/src/provider/services/jwt-token/jwt-token.service.tsapps/api/src/provider/services/jwt-token/jwt-token.service.spec.tsapps/api/src/provider/services/provider/provider.service.ts
📚 Learning: the `getbyowneranddseq` method in `apps/api/src/deployment/controllers/deployment/deployment.control...
Learnt from: baktun14
PR: akash-network/console#1428
File: apps/api/src/deployment/controllers/deployment/deployment.controller.ts:0-0
Timestamp: 2025-06-03T15:06:34.211Z
Learning: The `getByOwnerAndDseq` method in `apps/api/src/deployment/controllers/deployment/deployment.controller.ts` is intentionally public without the `@Protected` decorator because it serves public blockchain data from an indexer, following the pattern of public blockchain APIs.
Applied to files:
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts
📚 Learning: applies to **/*.spec.{ts,tsx} : don't use `jest.mock()` to mock dependencies in test files. instead,...
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test.
Applied to files:
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts
📚 Learning: applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : use `queryby` methods instead of `getb...
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-07-21T08:24:27.953Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Applied to files:
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts
📚 Learning: in apps/{deploy-web,provider-console}/**/*.spec.tsx files: use `getby` methods instead of `queryby` ...
Learnt from: stalniy
PR: akash-network/console#1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.711Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Applied to files:
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts
📚 Learning: applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it...
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Applied to files:
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts
📚 Learning: applies to **/*.spec.{ts,tsx} : use `setup` function instead of `beforeeach` in test files...
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Applied to files:
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts
📚 Learning: the getprovidershosturibyattributes method in apps/api/src/provider/repositories/provider/provider.r...
Learnt from: stalniy
PR: akash-network/console#1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Applied to files:
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts
🧬 Code Graph Analysis (5)
apps/api/src/deployment/services/lease/lease.service.ts (1)
apps/indexer/drizzle/schema.ts (1)
lease(110-178)
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts (2)
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts (2)
createWallet(59-69)createAndAuthorizeTrialSpending(44-57)apps/deploy-web/src/services/auth/auth.service.ts (1)
AuthService(5-21)
apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts (2)
apps/indexer/drizzle/schema.ts (3)
deployment(180-203)lease(110-178)provider(205-237)apps/api/src/deployment/http-schemas/deployment.schema.ts (1)
GetDeploymentResponse(252-252)
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts (2)
apps/api/src/billing/lib/wallet/wallet.ts (1)
Wallet(7-53)apps/api/src/billing/providers/config.provider.ts (1)
BillingConfig(9-9)
apps/api/src/provider/services/provider/provider.service.ts (4)
apps/api/src/billing/providers/config.provider.ts (2)
InjectBillingConfig(11-11)BillingConfig(9-9)apps/indexer/drizzle/schema.ts (1)
provider(205-237)apps/api/src/provider/services/provider/provider-proxy.service.ts (1)
ProviderIdentity(7-10)apps/api/src/deployment/http-schemas/lease.schema.ts (1)
LeaseStatusResponse(53-53)
🪛 GitHub Check: validate / validate-app
apps/api/src/provider/services/jwt-token/jwt-token.service.ts
[failure] 6-6:
Dependency cycle via "@src/billing/services/managed-signer/managed-signer.service:6=>../trial-validation/trial-validation.service:20=>@src/deployment/services/deployment-reader/deployment-reader.service:9=>@src/provider/services/provider/provider.service:12"
apps/api/src/provider/services/provider/provider.service.ts
[failure] 18-18:
Dependency cycle via "@src/billing/services:6=>@src/billing/services/managed-signer/managed-signer.service:6=>../trial-validation/trial-validation.service:20=>@src/deployment/services/deployment-reader/deployment-reader.service:9"
⏰ 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). (1)
- GitHub Check: test-build
🔇 Additional comments (17)
apps/api/src/billing/lib/wallet/wallet.ts (1)
50-52: LGTM! Clean method addition for JWT token support.The
getInstance()method provides a clean way to access the resolved wallet instance, which is needed for JWT token generation in the new authentication flow.apps/api/src/deployment/services/lease/lease.service.ts (1)
33-33: LGTM! Proper migration from certificate to JWT authentication.The change correctly replaces certificate-based authentication with wallet ID, which will be used to generate JWT tokens in the provider service.
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (1)
59-59: LGTM! Proper migration to wallet ID for JWT authentication.The change correctly replaces certificate-based authentication with wallet ID, which aligns with the JWT migration objectives.
apps/api/src/provider/services/jwt-token/jwt-token.service.ts (2)
14-31: LGTM! Well-structured JWT token generation logic.The JWT token generation implementation is solid:
- Proper use of the Akash wallet for signing
- Correct JWT claims structure with appropriate timing
- Good use of UUID for jti (JWT ID)
- Follows the established patterns from retrieved learnings
1-32: Ignore incorrect circular dependency warningAfter tracing the import graph through the following files, there is no cycle back to
managed-signer.serviceortrial-validation.service:•
managed-signer.service.tsdoes not importtrial-validation.service.ts.
•trial-validation.service.tsis only referenced by its own module (no upstream imports).
• The real chain isdeployment-reader.service.ts→provider.service.ts→jwt-token.service.ts→ billing providers/lib, with no return path.No refactoring is required. Please dismiss this warning.
Likely an incorrect or invalid review comment.
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts (4)
5-5: LGTM!The import addition for
JwtTokenServicecorrectly supports the JWT authentication migration described in the PR objectives.
98-112: LGTM!The
UserWalletRepositorymock updates provide comprehensive coverage with proper method mocking, including the chaining pattern foraccessibleByand correct value transformation intoPublic.
119-124: LGTM!The
JwtTokenServicemock registration properly supports the JWT authentication migration with appropriate method stubbing forgenerateJwtToken.
132-132: LGTM!The
SetupInputinterface extension with the optionalfindWalletByproperty maintains consistency with the mock structure updates.apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts (3)
58-102: LGTM!The
setupfunction follows coding guidelines correctly: positioned at the bottom of the describe block, uses inline type definition for parameters, returns objects without explicit return type annotation, and avoids shared state.
21-32: LGTM!The test cases for basic JWT token generation and different wallet IDs provide good coverage with proper assertions and mock verification.
Also applies to: 47-55
91-94: LGTM!Container registration and cleanup are handled correctly, following established patterns.
apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts (2)
104-104: LGTM!The method call correctly passes
wallet.idinstead of certificate parameters, aligning with the JWT authentication migration.
128-128: LGTM!The method signature and implementation changes correctly transition from certificate-based to JWT token-based authentication by:
- Accepting
walletIdparameter instead of certificate options- Passing the
walletIdtoproviderService.sendManifestThis aligns with the broader JWT authentication migration described in the PR objectives.
Also applies to: 131-131
apps/api/src/provider/services/provider/provider.service.ts (3)
13-13: LGTM!The import additions for
ProviderIdentity,ProviderProxyService, andJwtTokenServicecorrectly support the JWT authentication migration. The constructor parameter updates properly inject the required dependencies.Also applies to: 18-18, 32-33
38-38: LGTM!The method signature changes correctly transition from certificate-based to wallet ID-based authentication:
sendManifestandgetLeaseStatusnow acceptwalletIdparameter- Private
sendManifestToProvidermethod updated accordinglyThis supports the JWT token generation approach described in the PR objectives.
Also applies to: 82-82, 56-56
62-65: LGTM!The JWT token implementation correctly:
- Generates JWT tokens using the
JwtTokenServicewith wallet ID- Uses standard Bearer token format in Authorization header
- Sets appropriate Content-Type for JSON requests
This properly implements the JWT authentication described in the PR objectives.
Also applies to: 95-98
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts
Outdated
Show resolved
Hide resolved
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts
Outdated
Show resolved
Hide resolved
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts
Outdated
Show resolved
Hide resolved
|
This is great, thanks @jzsfkzm. A few things missing to consider this issue closed:
|
jzsfkzm
force-pushed
the
features/1038-jwt-not-cert
branch
from
75d3545 to
8401bd7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snappackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
apps/api/examples/lease-flow.ts(9 hunks)apps/api/package.json(1 hunks)apps/api/src/billing/lib/wallet/wallet.ts(1 hunks)apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts(3 hunks)apps/api/src/deployment/http-schemas/deployment.schema.ts(1 hunks)apps/api/src/deployment/http-schemas/lease.schema.ts(0 hunks)apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts(5 hunks)apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts(3 hunks)apps/api/src/deployment/services/lease/lease.service.ts(1 hunks)apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts(1 hunks)apps/api/src/provider/services/jwt-token/jwt-token.service.ts(1 hunks)apps/api/src/provider/services/provider/provider.service.ts(5 hunks)apps/api/test/functional/deployments.spec.ts(7 hunks)
💤 Files with no reviewable changes (1)
- apps/api/src/deployment/http-schemas/lease.schema.ts
✅ Files skipped from review due to trivial changes (1)
- apps/api/examples/lease-flow.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/api/package.json
- apps/api/src/billing/lib/wallet/wallet.ts
- apps/api/src/deployment/services/lease/lease.service.ts
- apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts
- apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts
- apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/deployment/http-schemas/deployment.schema.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/api/test/functional/deployments.spec.tsapps/api/src/provider/services/jwt-token/jwt-token.service.tsapps/api/src/provider/services/provider/provider.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/deployment/http-schemas/deployment.schema.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/api/test/functional/deployments.spec.tsapps/api/src/provider/services/jwt-token/jwt-token.service.tsapps/api/src/provider/services/provider/provider.service.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/test/functional/deployments.spec.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: baktun14
PR: akash-network/console#1312
File: packages/jwt/src/jwt-token.ts:0-0
Timestamp: 2025-05-13T17:35:25.875Z
Learning: In the Akash JWT implementation, cryptographic signature verification is handled by the provider node code rather than in the JWT package itself. The JWT package only handles token creation, decoding, and basic validation (schema and time constraints).
Learnt from: baktun14
PR: akash-network/console#1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
📚 Learning: the `getbyowneranddseq` method in `apps/api/src/deployment/controllers/deployment/deployment.control...
Learnt from: baktun14
PR: akash-network/console#1428
File: apps/api/src/deployment/controllers/deployment/deployment.controller.ts:0-0
Timestamp: 2025-06-03T15:06:34.211Z
Learning: The `getByOwnerAndDseq` method in `apps/api/src/deployment/controllers/deployment/deployment.controller.ts` is intentionally public without the `@Protected` decorator because it serves public blockchain data from an indexer, following the pattern of public blockchain APIs.
Applied to files:
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/api/test/functional/deployments.spec.ts
📚 Learning: applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : use `queryby` methods instead of `getb...
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-07-21T08:24:27.953Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Applied to files:
apps/api/test/functional/deployments.spec.ts
📚 Learning: applies to **/*.spec.{ts,tsx} : use `setup` function instead of `beforeeach` in test files...
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Applied to files:
apps/api/test/functional/deployments.spec.ts
📚 Learning: applies to **/*.spec.{ts,tsx} : don't use `jest.mock()` to mock dependencies in test files. instead,...
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test.
Applied to files:
apps/api/test/functional/deployments.spec.ts
📚 Learning: in apps/{deploy-web,provider-console}/**/*.spec.tsx files: use `getby` methods instead of `queryby` ...
Learnt from: stalniy
PR: akash-network/console#1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.711Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Applied to files:
apps/api/test/functional/deployments.spec.ts
📚 Learning: applies to **/*.spec.{ts,tsx} : don't use shared state in `setup` function...
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Applied to files:
apps/api/test/functional/deployments.spec.ts
📚 Learning: applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it...
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Applied to files:
apps/api/test/functional/deployments.spec.ts
📚 Learning: in the akash jwt implementation, cryptographic signature verification is handled by the provider nod...
Learnt from: baktun14
PR: akash-network/console#1312
File: packages/jwt/src/jwt-token.ts:0-0
Timestamp: 2025-05-13T17:35:25.875Z
Learning: In the Akash JWT implementation, cryptographic signature verification is handled by the provider node code rather than in the JWT package itself. The JWT package only handles token creation, decoding, and basic validation (schema and time constraints).
Applied to files:
apps/api/src/provider/services/jwt-token/jwt-token.service.tsapps/api/src/provider/services/provider/provider.service.ts
🧬 Code Graph Analysis (4)
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (2)
apps/api/src/deployment/http-schemas/deployment.schema.ts (1)
GetDeploymentResponse(248-248)apps/indexer/drizzle/schema.ts (2)
lease(110-178)deployment(180-203)
apps/api/test/functional/deployments.spec.ts (1)
apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts (1)
UserWalletOutput(16-20)
apps/api/src/provider/services/jwt-token/jwt-token.service.ts (2)
apps/api/src/provider/services/provider/provider.service.ts (1)
singleton(21-231)apps/api/src/billing/providers/config.provider.ts (2)
InjectBillingConfig(11-11)BillingConfig(9-9)
apps/api/src/provider/services/provider/provider.service.ts (4)
apps/api/src/billing/providers/config.provider.ts (2)
InjectBillingConfig(11-11)BillingConfig(9-9)apps/indexer/drizzle/schema.ts (1)
provider(205-237)apps/api/src/provider/services/provider/provider-proxy.service.ts (1)
ProviderIdentity(7-10)apps/api/src/deployment/http-schemas/lease.schema.ts (1)
LeaseStatusResponse(49-49)
🪛 GitHub Check: validate / validate-app
apps/api/src/provider/services/jwt-token/jwt-token.service.ts
[failure] 6-6:
Dependency cycle via "@src/billing/services/managed-signer/managed-signer.service:6=>../trial-validation/trial-validation.service:20=>@src/deployment/services/deployment-reader/deployment-reader.service:9=>@src/provider/services/provider/provider.service:12"
apps/api/src/provider/services/provider/provider.service.ts
[failure] 18-18:
Dependency cycle via "@src/billing/services:6=>@src/billing/services/managed-signer/managed-signer.service:6=>../trial-validation/trial-validation.service:20=>@src/deployment/services/deployment-reader/deployment-reader.service:9"
⏰ 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). (15)
- GitHub Check: validate (apps/indexer) / validate-unsafe
- GitHub Check: validate (apps/api) / validate-unsafe
- GitHub Check: validate (apps/provider-proxy) / validate-unsafe
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: 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: Socket Security: Pull Request Alerts
🔇 Additional comments (13)
apps/api/src/deployment/http-schemas/deployment.schema.ts (1)
102-106: LGTM! Certificate fields correctly removed.The simplification of
UpdateDeploymentRequestSchemato only require thesdlfield aligns perfectly with the migration from certificate-based to JWT-based authentication. The removal of the nested certificate object withcertPemandkeyPemfields is consistent with the broader refactor.apps/api/src/provider/services/jwt-token/jwt-token.service.ts (2)
10-12: LGTM! Proper singleton service structure.The service is correctly implemented as a singleton with appropriate dependency injection for the billing configuration.
14-31: JWT token generation implementation looks correct.The token generation follows proper JWT standards with appropriate claims structure for Akash network authentication. The use of ES256K signing with secp256k1 curve and the claims structure (iss, exp, nbf, iat, jti, version, leases) align with Akash JWT specifications.
apps/api/test/functional/deployments.spec.ts (3)
43-43: Good addition for wallet-based testing.The
allWalletsarray properly aggregates user wallets across tests to support the new wallet-based authentication flow.
71-73: Proper wallet repository mocking.The mock implementation correctly returns wallets by address, which aligns with how
DeploymentReaderServicenow fetches wallets by owner address instead of using certificates.
698-699: Certificate fields correctly removed from test payloads.The test request bodies now only include the
sdlfield, properly reflecting the removal of certificate requirements from the deployment update API. This maintains test coverage while adapting to the JWT-based authentication model.Also applies to: 725-726, 746-747, 770-771
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (4)
10-10: Proper dependency injection for wallet repository.The addition of
UserWalletRepositorydependency correctly enables wallet lookup functionality needed for the JWT-based authentication flow.Also applies to: 25-25
28-29: Clean method signature simplification.Removing the optional certificate parameter and using the helper method to fetch wallet information simplifies the method signature while maintaining functionality through the JWT-based approach.
48-48: Wallet ID correctly used for provider authentication.The switch from certificate-based to wallet ID-based provider authentication is properly implemented, using the wallet's ID to generate JWT tokens for lease status requests.
288-295: Well-implemented helper method with proper error handling.The
getWalletByAddresshelper method includes appropriate null checking and error handling, addressing the concern from previous reviews about potential null access.apps/api/src/provider/services/provider/provider.service.ts (3)
18-18: JWT token service properly integrated.The addition of
JwtTokenServicedependency enables JWT-based authentication for provider communications, replacing the previous certificate-based approach.Also applies to: 33-33
38-38: Method signatures correctly updated for wallet-based authentication.Both
sendManifestandgetLeaseStatusmethods now acceptwalletIdparameters instead of certificate options, which aligns with the JWT-based authentication model.Also applies to: 82-82
62-65: Proper JWT Bearer token authentication implementation.The HTTP headers correctly use the
Authorization: Bearer <token>format with JWT tokens generated from wallet IDs. TheContent-Type: application/jsonheader is appropriately set for JSON payloads.Also applies to: 95-98
jzsfkzm
force-pushed
the
features/1038-jwt-not-cert
branch
from
8401bd7 to
879a472
Compare
|
Also, I haven't done any benchmarks on jwt generation, but if it takes some time to generate a new one every time, we could keep a cache per address for like a few minutes. |
| Authorization: `Bearer ${await this.jwtTokenService.generateJwtToken(walletId)}`, | ||
| "Content-Type": "application/json" |
There was a problem hiding this comment.
question(blocking): provider proxy doesn't support JWT authentication. Do I miss something?
There was a problem hiding this comment.
with JWT, you don't need to use provider-proxy, you can directly talk to provider API
There was a problem hiding this comment.
Changed it and now it's communicating right with the provider, would you check?
jzsfkzm
force-pushed
the
features/1038-jwt-not-cert
branch
2 times, most recently
from
464301b to
a4cba21
Compare
| import { BillingConfig, InjectBillingConfig } from "@src/billing/providers"; | ||
| import { Memoize } from "@src/caching/helpers"; | ||
|
|
||
| const JWT_TOKEN_TTL_IN_SECONDS = 30; |
There was a problem hiding this comment.
suggestion(non-blocking): after merging changes to jwt, I'd like you to allow to pass this option into JwtTokenService with default to 30 secs or even less
There was a problem hiding this comment.
Made it configurable. I'd keep the 30s default, considering Rabbit's suggestion about network reasons. What do you think?
| exp: now + JWT_TOKEN_TTL_IN_SECONDS, | ||
| nbf: now, | ||
| iat: now, | ||
| jti: uuid.v4(), |
There was a problem hiding this comment.
question(non-blocking): what do you think about using token prefixes as github does?
There was a problem hiding this comment.
Do you mean prefixing the jti field? Should I add a param for that to the method? What would be the use of that? Should it have a default?
jzsfkzm
force-pushed
the
features/1038-jwt-not-cert
branch
from
a4cba21 to
9deffa0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
apps/api/src/provider/services/provider/provider.service.ts (2)
74-80: Do not useany; improve error handling with type guards.This violates the TS guideline forbidding
any. Also, usingerr.messageon an unknown shape is unsafe and can misreport Axios error payloads.Apply:
+import { isAxiosError } from "axios"; @@ - } catch (err: any) { - if (err.message?.includes("no lease for deployment") && i < this.MANIFEST_SEND_MAX_RETRIES) { + } catch (err: unknown) { + const message = + isAxiosError(err) + ? (typeof err.response?.data === "string" ? err.response.data : err.message) + : (err instanceof Error ? err.message : String(err)); + if (message?.includes("no lease for deployment") && i < this.MANIFEST_SEND_MAX_RETRIES) { await delay(this.MANIFEST_SEND_RETRY_DELAY); continue; } - throw new Error(err?.response?.data || err); + throw new Error(message); }
31-47: Ensurelease-flow.tsuses JWT instead of API keys
The example atapps/api/examples/lease-flow.tsno longer references certificates—which is good—but it still authenticates with an API key (x-api-key) rather than a JWT. To complete the migration:
- File to update:
apps/api/examples/lease-flow.ts- In each call that sets
replace with something like:headers: { "x-api-key": apiKey }headers: { Authorization: `Bearer ${jwtToken}` }- Add a step at the top of the script to obtain or load
JWT_TOKEN(e.g. call your auth endpoint or read from env)- Remove the
API_KEYenv var references and related setupThis will close the issue by fully adopting JWT flows in the example.
♻️ Duplicate comments (1)
apps/api/src/provider/services/jwt-token/jwt-token.service.ts (1)
11-12: TTL increased to 30s — good balance for network latency and retries.This addresses earlier concerns about too-short tokens. Keep the ability to override via the method param.
🧹 Nitpick comments (4)
apps/api/src/provider/services/jwt-token/jwt-token.service.ts (2)
28-41: Account for small clock skew and guard TTL.To reduce “token not yet valid” edge cases, offset nbf by a few seconds and ensure
ttlis at least 1.Apply:
- const now = Math.floor(Date.now() / 1000); + const now = Math.floor(Date.now() / 1000); + const skew = 5; // seconds const token = await jwtToken.createToken({ iss: address, - exp: now + ttl, - nbf: now, + exp: now + Math.max(1, ttl), + nbf: now - skew, iat: now, jti: uuid.v4(), version: "v1", leases });
18-22: Consider typingleasesas JWTTokenPayload['leases'] (post jwt types update).Once the improved types are released in the jwt package, aligning to the payload type clarifies intent and reduces mismatch risk.
apps/api/src/provider/services/provider/provider.service.ts (2)
31-33: Avoid string-based JSON mutation for manifest.Regex replace on a JSON string is fragile. Prefer parsing + transforming the object, then
JSON.stringify.If out of scope, add a TODO to replace this with a structured transformation.
84-99: Consistent parameter shape: prefer object params forgetLeaseStatus.
sendManifestuses an object param;getLeaseStatusstill uses positional args. For consistency and future extensibility, consider switching to an object param (tracked as a separate issue to avoid widening PR scope).Would you like me to open an issue proposing this API shape refactor?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snappackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
apps/api/examples/lease-flow.ts(9 hunks)apps/api/package.json(1 hunks)apps/api/src/billing/lib/wallet/wallet.ts(1 hunks)apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts(3 hunks)apps/api/src/deployment/http-schemas/deployment.schema.ts(1 hunks)apps/api/src/deployment/http-schemas/lease.schema.ts(0 hunks)apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts(5 hunks)apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts(3 hunks)apps/api/src/deployment/services/lease/lease.service.ts(1 hunks)apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts(1 hunks)apps/api/src/provider/services/jwt-token/jwt-token.service.ts(1 hunks)apps/api/src/provider/services/provider/provider.service.spec.ts(1 hunks)apps/api/src/provider/services/provider/provider.service.ts(3 hunks)apps/api/test/functional/deployments.spec.ts(7 hunks)packages/http-sdk/src/provider/provider-http.service.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/api/src/deployment/http-schemas/lease.schema.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/api/package.json
- apps/api/src/billing/lib/wallet/wallet.ts
- apps/api/examples/lease-flow.ts
- apps/api/src/deployment/http-schemas/deployment.schema.ts
- apps/api/src/deployment/services/lease/lease.service.ts
- apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts
- packages/http-sdk/src/provider/provider-http.service.ts
- apps/api/test/functional/deployments.spec.ts
- apps/api/src/provider/services/provider/provider.service.spec.ts
- apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts
- apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts
- apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/provider/services/jwt-token/jwt-token.service.tsapps/api/src/provider/services/provider/provider.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/provider/services/jwt-token/jwt-token.service.tsapps/api/src/provider/services/provider/provider.service.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: baktun14
PR: akash-network/console#1312
File: packages/jwt/src/jwt-token.ts:0-0
Timestamp: 2025-05-13T17:35:25.875Z
Learning: In the Akash JWT implementation, cryptographic signature verification is handled by the provider node code rather than in the JWT package itself. The JWT package only handles token creation, decoding, and basic validation (schema and time constraints).
Learnt from: baktun14
PR: akash-network/console#1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
📚 Learning: 2025-05-13T17:35:25.875Z
Learnt from: baktun14
PR: akash-network/console#1312
File: packages/jwt/src/jwt-token.ts:0-0
Timestamp: 2025-05-13T17:35:25.875Z
Learning: In the Akash JWT implementation, cryptographic signature verification is handled by the provider node code rather than in the JWT package itself. The JWT package only handles token creation, decoding, and basic validation (schema and time constraints).
Applied to files:
apps/api/src/provider/services/jwt-token/jwt-token.service.tsapps/api/src/provider/services/provider/provider.service.ts
📚 Learning: 2025-07-24T17:00:52.361Z
Learnt from: baktun14
PR: akash-network/console#1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
Applied to files:
apps/api/src/provider/services/jwt-token/jwt-token.service.ts
📚 Learning: 2025-06-08T03:07:13.871Z
Learnt from: stalniy
PR: akash-network/console#1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Applied to files:
apps/api/src/provider/services/provider/provider.service.ts
📚 Learning: 2025-06-03T15:06:34.211Z
Learnt from: baktun14
PR: akash-network/console#1428
File: apps/api/src/deployment/controllers/deployment/deployment.controller.ts:0-0
Timestamp: 2025-06-03T15:06:34.211Z
Learning: The `getByOwnerAndDseq` method in `apps/api/src/deployment/controllers/deployment/deployment.controller.ts` is intentionally public without the `Protected` decorator because it serves public blockchain data from an indexer, following the pattern of public blockchain APIs.
Applied to files:
apps/api/src/provider/services/provider/provider.service.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: validate (apps/provider-proxy) / validate-unsafe
- 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: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (2)
apps/api/src/provider/services/jwt-token/jwt-token.service.ts (1)
45-52: Memoized signer/token scaffolding is appropriate.Caching the signer/JwtToken per wallet for 5 minutes reduces overhead without violating short token TTL constraints.
apps/api/src/provider/services/provider/provider.service.ts (1)
62-70: JWT scoping looks correct for send-manifest.Granular, provider-scoped permission for "send-manifest" aligns with the intended least-privilege approach.
| import { ProviderIdentity } from "@src/provider/services/provider/provider-proxy.service"; | ||
| import { ProviderList } from "@src/types/provider"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove dependency on provider-proxy type; define a local minimal type.
ProviderIdentity import from provider-proxy ties this service to a deprecated path. Define a local type here to avoid accidental coupling.
Apply:
-import { ProviderIdentity } from "@src/provider/services/provider/provider-proxy.service";
+type ProviderIdentity = { owner: string; hostUri: string };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { ProviderIdentity } from "@src/provider/services/provider/provider-proxy.service"; | |
| import { ProviderList } from "@src/types/provider"; | |
| type ProviderIdentity = { owner: string; hostUri: string }; | |
| import { ProviderList } from "@src/types/provider"; |
🤖 Prompt for AI Agents
In apps/api/src/provider/services/provider/provider.service.ts at lines 11 to
12, remove the import of ProviderIdentity from the deprecated
provider-proxy.service path. Instead, define a minimal local type for
ProviderIdentity within this file to avoid coupling to the deprecated module.
Ensure the local type includes only the necessary properties used in this
service.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/test/functional/lease-flow.spec.ts (1)
132-147: Skip certificate creation whenincludeCertificateis false
runLifecycle()always calls/v1/certificates, even when the test variant is supposed to run without a certificate. This adds ~3 network calls and blurs the intention of the “no-certificate” scenario.- // 3. Create certificate - const certResponse = await app.request("/v1/certificates", { … }); - … + let certPem: string | undefined; + let encryptedKey: string | undefined; + if (includeCertificate) { + const certResponse = await app.request("/v1/certificates", { … }); + ({ certPem, encryptedKey } = (await certResponse.json()).data); + }This keeps the happy-path fast and makes the two variants semantically distinct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/test/functional/deployments.spec.ts(7 hunks)apps/api/test/functional/lease-flow.spec.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/test/functional/deployments.spec.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/test/functional/lease-flow.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/test/functional/lease-flow.spec.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/test/functional/lease-flow.spec.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: baktun14
PR: akash-network/console#1312
File: packages/jwt/src/jwt-token.ts:0-0
Timestamp: 2025-05-13T17:35:25.875Z
Learning: In the Akash JWT implementation, cryptographic signature verification is handled by the provider node code rather than in the JWT package itself. The JWT package only handles token creation, decoding, and basic validation (schema and time constraints).
Learnt from: baktun14
PR: akash-network/console#1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Applied to files:
apps/api/test/functional/lease-flow.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Applied to files:
apps/api/test/functional/lease-flow.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Applied to files:
apps/api/test/functional/lease-flow.spec.ts
📚 Learning: 2025-07-21T08:24:27.953Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-07-21T08:24:27.953Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Applied to files:
apps/api/test/functional/lease-flow.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Applied to files:
apps/api/test/functional/lease-flow.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block in test files
Applied to files:
apps/api/test/functional/lease-flow.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Applied to files:
apps/api/test/functional/lease-flow.spec.ts
📚 Learning: 2025-07-21T08:24:24.269Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test.
Applied to files:
apps/api/test/functional/lease-flow.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (1)
apps/api/test/functional/lease-flow.spec.ts (1)
117-130: Prefertest.each& migrate away frombeforeEach/shared stateThe new parameterisation works, but:
- Jest already provides
test.each/describe.each, which yields cleaner output and eliminates the manual loop.- Project guidelines for
*.spec.tsrequire using asetup()helper instead ofbeforeEachand shared top-level variables. Migrating to that pattern here will bring the file in line with the rest of the suite and avoid hidden state between runs.Refactoring now keeps the file future-proof and consistent.
⛔ Skipped due to learnings
Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-07-21T08:25:07.474Z Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test filesLearnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-07-21T08:25:07.474Z Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` functionLearnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-07-21T08:25:07.474Z Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block in test filesLearnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-07-21T08:25:07.474Z Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns itLearnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-07-21T08:25:07.474Z Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definitionLearnt from: CR PR: akash-network/console#0 File: .cursor/rules/no-jest-mock.mdc:0-0 Timestamp: 2025-07-21T08:24:24.269Z Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test.Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-07-21T08:25:07.474Z Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` functionLearnt from: CR PR: akash-network/console#0 File: .cursor/rules/query-by-in-tests.mdc:0-0 Timestamp: 2025-07-21T08:24:27.953Z Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
jzsfkzm
force-pushed
the
features/1038-jwt-not-cert
branch
from
b258616 to
64c64f3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (4)
apps/api/examples/lease-flow.ts (1)
256-256: Certificate fields still being sent in deployment update.The deployment update request at line 256 still includes certificate fields (
certPemandkeyPem), which contradicts the PR objective of removing certificate-based authentication.Remove the certificate fields from the update request:
- body: JSON.stringify({ data: { sdl: ymlUpdate, certificate: { certPem, keyPem: encryptedKey } } }) + body: JSON.stringify({ data: { sdl: ymlUpdate } })apps/api/test/functional/lease-flow.spec.ts (1)
256-256: Certificate still being sent in deployment update request.Similar to the example file, the deployment update request still includes certificate fields even when
includeCertificateis false, which doesn't align with the test's intention to validate certificate-less flow.Update the deployment update request to conditionally include certificate:
- body: JSON.stringify({ data: { sdl: ymlUpdate, certificate: { certPem, keyPem: encryptedKey } } }) + body: JSON.stringify({ data: { sdl: ymlUpdate, certificate: includeCertificate ? { certPem, keyPem: encryptedKey } : undefined } })apps/api/test/functional/deployments.spec.ts (1)
713-741: Legacy certificate field in payload contradicts JWT migration intent; rename and drop the field, or remove the test
- The PR’s goal is to remove certificates from endpoints. Keeping a positive test that sends certificate encourages legacy usage.
- Also, the AI summary claims the test sends only sdl, but the code includes a certificate object.
Consider renaming the test to clarify backward-compat behavior and removing the field to avoid duplication with the previous test. Apply:
- it("should update a deployment successfully with a certificate provided", async () => { + it("ignores legacy certificate field in payload (backward-compat)", async () => { @@ - body: JSON.stringify({ - data: { - sdl: yml, - certificate: { - certPem: "test-cert-pem", - keyPem: "test-key-pem" - } - } - }), + body: JSON.stringify({ + data: { + sdl: yml + } + }),Alternatively, if the API should reject legacy fields now, delete this test entirely and add a negative test asserting 400 on unknown fields (depending on schema strictness).
apps/api/src/provider/services/provider/provider.service.ts (1)
75-80: Do not useany; catch unknown and preserve error details safelyPer coding guidelines, avoid
any. Also, preserve useful context when rethrowing.Apply:
- } catch (err: any) { - if (err.message?.includes("no lease for deployment") && i < this.MANIFEST_SEND_MAX_RETRIES) { + } catch (err: unknown) { + const msg = + typeof err === "object" && err !== null && "message" in err + ? String((err as Record<string, unknown>).message) + : String(err); + if (msg.includes("no lease for deployment") && i < this.MANIFEST_SEND_MAX_RETRIES) { await delay(this.MANIFEST_SEND_RETRY_DELAY); continue; } - throw new Error(err?.response?.data || err); + const maybeResponse = (err as Record<string, unknown>)["response"]; + const maybeData = + typeof maybeResponse === "object" && maybeResponse !== null && "data" in maybeResponse + ? (maybeResponse as Record<string, unknown>)["data"] + : undefined; + throw new Error(maybeData ? JSON.stringify(maybeData) : msg); }
♻️ Duplicate comments (1)
apps/api/src/provider/services/provider/provider.service.ts (1)
11-11: Remove import from deprecated provider-proxy; define a minimal local typeThis was raised earlier and still applies. Uncouple from provider-proxy by defining a local type.
Apply:
-import { ProviderIdentity } from "@src/provider/services/provider/provider-proxy.service"; +type ProviderIdentity = { owner: string; hostUri: string };
🧹 Nitpick comments (14)
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts (4)
19-19: Consider renaming for clarity.The variable name
findWalletByis generic and doesn't clearly indicate it mocksfindOneBy. Consider renaming tofindOneByfor better alignment with the actual method name.- const findWalletBy = jest.fn().mockImplementation(async () => newWallet); + const findOneBy = jest.fn().mockImplementation(async () => newWallet);
26-26: Update parameter name to match the renamed mock.If the mock variable is renamed to
findOneBy, update the parameter accordingly.- findWalletBy, + findOneBy,
132-132: Update interface property name for consistency.If the mock variable is renamed to
findOneBy, update the interface property accordingly.- findWalletBy?: UserWalletRepository["findOneBy"]; + findOneBy?: UserWalletRepository["findOneBy"];
99-99: Update mock assignment to use consistent naming.If the interface property is renamed, update the mock assignment accordingly.
- findOneBy: input?.findWalletBy, + findOneBy: input?.findOneBy,apps/api/examples/lease-flow.ts (2)
5-5: Update example description to reflect JWT authentication.The comment states "Checks if API key env var is set" but this doesn't accurately describe the transition from certificate-based to JWT-based authentication, which is the main objective of this PR.
Update the comment to better reflect the new authentication approach:
- * 1. Checks if API key env var is set + * 1. Validates API key for authentication
71-71: Update script description to mention JWT authentication.The docstring should be updated to reflect that the example now demonstrates JWT-based authentication flow instead of certificate-based flow.
- * It creates a deployment, waits for bids, creates a lease, and then closes the deployment. + * It creates a deployment, waits for bids, creates a lease using JWT authentication, and then closes the deployment.apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (2)
86-95: Consider error handling for lease status fetching in bulk operations.While the individual lease status calls are wrapped in try/catch blocks in
findByOwnerAndDseq, the bulk operations inlistmethod don't have similar error handling. If one lease status fetch fails, it could cause the entire operation to fail.Add error handling for individual lease status fetches:
- leaseResults.map(async ({ leases }) => { - return await Promise.all( - leases.map(async ({ lease }) => { - return await this.providerService.getLeaseStatus(lease.lease_id.provider, lease.lease_id.dseq, lease.lease_id.gseq, lease.lease_id.oseq, wallet.id); - }) - ); - }) + leaseResults.map(async ({ leases }) => { + return await Promise.all( + leases.map(async ({ lease }) => { + try { + return await this.providerService.getLeaseStatus(lease.lease_id.provider, lease.lease_id.dseq, lease.lease_id.gseq, lease.lease_id.oseq, wallet.id); + } catch { + return null; + } + }) + ); + })
288-295: Consider caching wallet lookups to avoid redundant database queries.The
getWalletByAddressmethod is called multiple times for the same address across different operations. For better performance, consider implementing a short-lived cache for wallet lookups.Consider implementing a caching mechanism for wallet lookups to reduce database queries, especially when multiple operations are performed for the same address in quick succession. This could be done using a simple Map with TTL or a dedicated caching solution.
apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts (1)
126-131: Consider adding retry logic for provider manifest delivery.The
sendManifestToProvidersmethod sends manifests sequentially without retry logic. If a provider is temporarily unreachable, the manifest delivery will fail without retry.Consider adding retry logic with exponential backoff:
private async sendManifestToProviders(walletId: number, dseq: string, manifest: string, leases: GetDeploymentResponse["data"]["leases"]): Promise<void> { const leaseProviders = leases.map(lease => lease.lease_id.provider).filter((v, i, s) => s.indexOf(v) === i); for (const provider of leaseProviders) { - await this.providerService.sendManifest({ provider, dseq, manifest, walletId }); + let retries = 3; + let delay = 1000; + while (retries > 0) { + try { + await this.providerService.sendManifest({ provider, dseq, manifest, walletId }); + break; + } catch (error) { + retries--; + if (retries === 0) throw error; + await new Promise(resolve => setTimeout(resolve, delay)); + delay *= 2; + } + } } }apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts (1)
105-161: Consider using the setup function pattern as recommended.According to the coding guidelines, the
setupfunction should be at the bottom of the rootdescribeblock. Currently it's placed after the tests.Move the setup function to the bottom of the describe block:
describe("JwtTokenService", () => { const mockMnemonic = "test mnemonic phrase for testing purposes only"; const mockAddress = "akash1testaddress123456789"; describe("generateJwtToken", () => { // ... tests ... }); describe("getGranularLeases", () => { // ... tests ... }); + + function setup(): { + // ... setup implementation ... + } }); - - function setup(): { - // ... setup implementation ... - }apps/api/test/functional/deployments.spec.ts (2)
71-73: Harden findOneBy mock against undefined query inputsMinor guard to avoid accidental throws if a caller passes an empty or incompatible query.
Apply this diff:
- jest.spyOn(userWalletRepository, "findOneBy").mockImplementation(async (query: Partial<UserWalletOutput> | undefined) => { - return Promise.resolve(allWallets.find(wallet => wallet.address === query?.address)); - }); + jest.spyOn(userWalletRepository, "findOneBy").mockImplementation(async (query?: Partial<UserWalletOutput>) => { + if (!query?.address) return undefined; + return allWallets.find(wallet => wallet.address === query.address); + });
46-51: Test style deviates from repo guidelines (uses beforeEach instead of setup pattern)Per coding guidelines for .spec.ts files, prefer a setup function at the bottom of the root describe instead of beforeEach. If functional tests are exempt, ignore; otherwise, consider refactoring in a follow-up to align styles.
apps/api/src/provider/services/provider/provider.service.ts (2)
31-33: Fragile manifest string replace risks corrupting payloadReplacing "quantity" with "size" via a global regex on a serialized manifest is brittle and may produce invalid JSON or unintended replacements. If this transform is still required, prefer an AST-level transform.
Would you like me to propose a safe transform that parses and serializes the manifest, remapping only the intended fields?
49-59: Avoid re-generating JWT inside retry loopJWT creation is deterministic for the same wallet/scope and adds overhead on each retry. Generate once before looping.
Apply:
- }) { - for (let i = 1; i <= this.MANIFEST_SEND_MAX_RETRIES; i++) { - try { - const jwtToken = await this.jwtTokenService.generateJwtToken({ - walletId, - leases: this.jwtTokenService.getGranularLeases({ - provider: providerIdentity.owner, - scope: ["send-manifest"] - }) - }); - const result = await this.providerHttpService.sendManifest({ hostUri: providerIdentity.hostUri, dseq, manifest, jwtToken }); + }) { + const jwtToken = await this.jwtTokenService.generateJwtToken({ + walletId, + leases: this.jwtTokenService.getGranularLeases({ + provider: providerIdentity.owner, + scope: ["send-manifest"] + }) + }); + + for (let i = 1; i <= this.MANIFEST_SEND_MAX_RETRIES; i++) { + try { + const result = await this.providerHttpService.sendManifest({ hostUri: providerIdentity.hostUri, dseq, manifest, jwtToken });Also applies to: 62-70
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snappackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
apps/api/examples/lease-flow.ts(9 hunks)apps/api/package.json(1 hunks)apps/api/src/billing/lib/wallet/wallet.ts(1 hunks)apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts(3 hunks)apps/api/src/deployment/http-schemas/deployment.schema.ts(1 hunks)apps/api/src/deployment/http-schemas/lease.schema.ts(0 hunks)apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts(5 hunks)apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts(3 hunks)apps/api/src/deployment/services/lease/lease.service.ts(1 hunks)apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts(1 hunks)apps/api/src/provider/services/jwt-token/jwt-token.service.ts(1 hunks)apps/api/src/provider/services/provider/provider.service.spec.ts(1 hunks)apps/api/src/provider/services/provider/provider.service.ts(3 hunks)apps/api/test/functional/deployments.spec.ts(7 hunks)apps/api/test/functional/lease-flow.spec.ts(3 hunks)packages/http-sdk/src/provider/provider-http.service.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/api/src/deployment/http-schemas/lease.schema.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/api/package.json
- apps/api/src/billing/lib/wallet/wallet.ts
- apps/api/src/deployment/services/lease/lease.service.ts
- apps/api/src/deployment/http-schemas/deployment.schema.ts
- packages/http-sdk/src/provider/provider-http.service.ts
- apps/api/src/provider/services/provider/provider.service.spec.ts
- apps/api/src/provider/services/jwt-token/jwt-token.service.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.tsapps/api/test/functional/lease-flow.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/test/functional/deployments.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.tsapps/api/test/functional/lease-flow.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/examples/lease-flow.tsapps/api/src/deployment/services/deployment-writer/deployment-writer.service.tsapps/api/src/provider/services/provider/provider.service.tsapps/api/test/functional/deployments.spec.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.tsapps/api/test/functional/lease-flow.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/examples/lease-flow.tsapps/api/src/deployment/services/deployment-writer/deployment-writer.service.tsapps/api/src/provider/services/provider/provider.service.tsapps/api/test/functional/deployments.spec.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: baktun14
PR: akash-network/console#1312
File: packages/jwt/src/jwt-token.ts:0-0
Timestamp: 2025-05-13T17:35:25.875Z
Learning: In the Akash JWT implementation, cryptographic signature verification is handled by the provider node code rather than in the JWT package itself. The JWT package only handles token creation, decoding, and basic validation (schema and time constraints).
📚 Learning: 2025-07-21T08:24:24.269Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test.
Applied to files:
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts
📚 Learning: 2025-06-03T15:06:34.211Z
Learnt from: baktun14
PR: akash-network/console#1428
File: apps/api/src/deployment/controllers/deployment/deployment.controller.ts:0-0
Timestamp: 2025-06-03T15:06:34.211Z
Learning: The `getByOwnerAndDseq` method in `apps/api/src/deployment/controllers/deployment/deployment.controller.ts` is intentionally public without the `Protected` decorator because it serves public blockchain data from an indexer, following the pattern of public blockchain APIs.
Applied to files:
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts
🧬 Code Graph Analysis (6)
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts (2)
apps/api/src/billing/providers/config.provider.ts (1)
BillingConfig(9-9)apps/api/src/billing/lib/wallet/wallet.ts (1)
Wallet(7-53)
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts (1)
apps/api/src/billing/services/managed-user-wallet/managed-user-wallet.service.ts (2)
createWallet(59-69)createAndAuthorizeTrialSpending(44-57)
apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts (2)
apps/indexer/drizzle/schema.ts (3)
deployment(180-203)lease(110-178)provider(205-237)apps/api/src/deployment/http-schemas/deployment.schema.ts (1)
GetDeploymentResponse(248-248)
apps/api/src/provider/services/provider/provider.service.ts (6)
apps/api/src/provider/services/jwt-token/jwt-token.service.ts (1)
singleton(24-60)apps/api/src/deployment/services/lease/lease.service.ts (1)
singleton(10-38)apps/api/src/deployment/services/cached-balance/cached-balance.service.ts (1)
singleton(21-37)packages/http-sdk/src/provider/provider-http.service.ts (1)
ProviderHttpService(6-42)apps/api/src/provider/services/provider/provider-proxy.service.ts (1)
ProviderIdentity(7-10)apps/api/src/deployment/http-schemas/lease.schema.ts (1)
LeaseStatusResponse(49-49)
apps/api/test/functional/deployments.spec.ts (1)
apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts (1)
UserWalletOutput(16-20)
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (2)
apps/api/src/deployment/http-schemas/deployment.schema.ts (1)
GetDeploymentResponse(248-248)apps/indexer/drizzle/schema.ts (2)
lease(110-178)deployment(180-203)
⏰ 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/provider-console) / validate-unsafe
- GitHub Check: validate (apps/indexer) / validate-unsafe
- GitHub Check: validate (apps/notifications) / validate-unsafe
- GitHub Check: validate (packages) / validate-unsafe
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (13)
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts (3)
5-5: LGTM! JWT service properly imported for dependency injection.The import is correctly added to support the new JWT-based authentication system.
98-112: Well-structured UserWalletRepository mock implementation.The mock properly implements the repository pattern with correct method signatures and the
accessibleBymethod returning the mock instance itself, which is a good pattern for chainable methods.
119-124: LGTM! JWT token service mock properly configured.The mock correctly provides the
generateJwtTokenmethod returning a mock token string, which aligns with the JWT authentication migration objectives.apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (1)
28-49: LGTM! Clean implementation of wallet-based lease status retrieval.The migration from certificate-based to wallet-based authentication is well implemented. The method now properly retrieves the wallet and uses its ID for lease status lookups, with appropriate error handling.
apps/api/test/functional/lease-flow.spec.ts (1)
117-130: Comprehensive test coverage for both certificate paths.Excellent approach to test both certificate-present and certificate-absent scenarios using parameterized tests. This ensures backward compatibility while validating the new JWT-based flow.
apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts (1)
94-107: LGTM! Clean transition to wallet-based manifest delivery.The update method has been successfully refactored to use wallet ID for JWT authentication instead of certificates. The manifest sending logic is properly abstracted.
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts (2)
1-14: Well-structured test setup following coding guidelines.The test file properly uses
jest-mock-extendedfor creating mocks and avoids usingjest.mock(), adhering to the coding guidelines.
50-59: Excellent test for JWT memoization.Great test coverage for verifying that JWT token generation is properly memoized, ensuring that wallet creation and signing operations are not repeated unnecessarily.
apps/api/test/functional/deployments.spec.ts (4)
43-45: Wallet aggregation + findOneBy mock are sensible and scoped. LGTMCollecting wallets in allWallets and resolving findOneBy by address keeps tests deterministic once reset in beforeEach. This reduces duplication and mirrors the repository’s typical usage.
Also applies to: 50-51, 71-73, 117-118
694-711: PUT update without certificate looks correctThe test reflects the JWT migration by sending only sdl and asserting deployment info + leases are returned. Good coverage.
760-766: 404 path without certificate aligns with new contractUpdating the negative path to only include sdl keeps tests aligned with JWT-based flow.
785-786: Input fields trimmed down to sdl for unauthorized and invalid-sdl cases is correctThese cases no longer reference certificate fields and focus on the intended validation.
Also applies to: 809-810
apps/api/src/provider/services/provider/provider.service.ts (1)
84-99: Lease status via JWT and direct provider HTTP is correctSwitching to JWT-scoped "status" and calling provider directly matches the new architecture. Looks good.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/src/provider/services/provider/provider.service.spec.ts (1)
38-38: Consider extracting granular leases creation to a helper.The
getGranularLeasesmock setup is duplicated across tests. Consider creating a test helper function to generate consistent lease configurations and reduce duplication.+ function createGranularLeases(providerAddress: string, scope: string[]): JwtTokenPayload["leases"] { + return { + access: "granular", + permissions: [{ provider: providerAddress, access: "scoped", scope }] + }; + } + function setup() { const providerHttpService = mock<ProviderHttpService>(); const providerAttributesSchemaService = mock<ProviderAttributesSchemaService>(); const auditorsService = mock<AuditorService>(); const jwtTokenService = mock<JwtTokenService>(); const service = new ProviderService(providerHttpService, providerAttributesSchemaService, auditorsService, jwtTokenService); return { service, providerHttpService, providerAttributesSchemaService, auditorsService, - jwtTokenService + jwtTokenService, + createGranularLeases }; }Also applies to: 190-190
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts(1 hunks)apps/api/src/provider/services/jwt-token/jwt-token.service.ts(1 hunks)apps/api/src/provider/services/provider/provider.service.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts
- apps/api/src/provider/services/jwt-token/jwt-token.service.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/provider/services/provider/provider.service.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/provider/services/provider/provider.service.spec.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/provider/services/provider/provider.service.spec.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: baktun14
PR: akash-network/console#1312
File: packages/jwt/src/jwt-token.ts:0-0
Timestamp: 2025-05-13T17:35:25.875Z
Learning: In the Akash JWT implementation, cryptographic signature verification is handled by the provider node code rather than in the JWT package itself. The JWT package only handles token creation, decoding, and basic validation (schema and time constraints).
📚 Learning: 2025-06-08T03:07:13.871Z
Learnt from: stalniy
PR: akash-network/console#1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Applied to files:
apps/api/src/provider/services/provider/provider.service.spec.ts
🧬 Code Graph Analysis (1)
apps/api/src/provider/services/provider/provider.service.spec.ts (1)
packages/http-sdk/src/provider/provider-http.service.ts (1)
ProviderHttpService(6-42)
⏰ 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 (apps/stats-web) / validate-unsafe
- GitHub Check: validate (apps/notifications) / validate-unsafe
- 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: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (6)
apps/api/src/provider/services/provider/provider.service.spec.ts (6)
1-9: LGTM! Clean imports and proper TypeScript types.The imports follow best practices with type-only imports where appropriate and proper use of
jest-mock-extendedfor mocking as specified in the coding guidelines.
225-241: LGTM! Proper test setup following coding guidelines.The
setupfunction correctly:
- Uses
jest-mock-extendedfor mocking dependencies- Is placed at the bottom of the describe block
- Accepts a single parameter (none in this case, which is fine)
- Doesn't use shared state
- Returns the service and its dependencies for testing
99-125: Retry limit is defined in codeThe service uses a private constant
MANIFEST_SEND_MAX_RETRIES = 3(apps/api/src/provider/services/provider/provider.service.ts line 21), which matches the test’s expectation of three attempts. No external configuration exists for this value—any change to the retry limit must be updated here and in the corresponding spec.• Location of retry limit:
• apps/api/src/provider/services/provider/provider.service.ts, line 21
12-55: Quantity→Size Replacement Is Intentional and Implemented
Inapps/api/src/provider/services/provider/provider.service.ts, thesendManifestmethod clearly applies:const manifestWithSize = manifest.replace(/"quantity":{"val/g, '"size":{"val');This matches the test’s expectation of transforming
{"quantity":{"val":"1"}}to{"size":{"val":"1"}}. No changes required.
31-34: Scopes align with JWT schema and implementation – no changes neededVerified that both
"send-manifest"and"status"are listed in packages/jwt/src/types.ts and the generated JSON schema, and that provider.service.ts correctly uses"send-manifest"for sending manifests and"status"for lease status. Tests accordingly pass the right scopes.
57-84: Retry logic confirmed & test timeout verified
- In
apps/api/src/provider/services/provider/provider.service.ts:
MANIFEST_SEND_MAX_RETRIES = 3MANIFEST_SEND_RETRY_DELAY = 6000(ms)- Retry loop on “no lease for deployment” errors runs at most once (first call fails, waits 6 s, then succeeds), fitting comfortably within the 10 s Jest timeout.
No changes required.
jzsfkzm
force-pushed
the
features/1038-jwt-not-cert
branch
from
d64e7b2 to
3f93180
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (3)
apps/api/test/functional/lease-flow.spec.ts (1)
256-256: Inconsistency: Certificate still included in update request.While the lease creation conditionally includes the certificate based on
includeCertificate, the update request always includes it. This seems inconsistent with the goal of supporting certificate-less flows.The update request should also respect the
includeCertificateparameter:-body: JSON.stringify({ data: { sdl: ymlUpdate, certificate: { certPem, keyPem: encryptedKey } } }) +body: JSON.stringify({ + data: { + sdl: ymlUpdate, + ...(includeCertificate && { certificate: { certPem, keyPem: encryptedKey } }) + } +})apps/api/test/functional/deployments.spec.ts (1)
713-751: Remove legacy "with certificate provided" test from deployments.spec.tsUpdateDeploymentRequestSchema no longer accepts a certificate (it only contains
sdl), so the test that sends a certificate is stale and should be removed or replaced with an explicit test for unknown-field handling at the validator layer.Files to update:
- apps/api/test/functional/deployments.spec.ts — remove the
it("should update a deployment successfully with a certificate provided", ...)test (lines ~713–751).- apps/api/src/deployment/http-schemas/deployment.schema.ts — verified:
UpdateDeploymentRequestSchemaonly includessdl.- apps/api/test/functional/lease-flow.spec.ts — contains other tests that include
certificate; review/adjust those tests for parity with the new schema.Apply:
- it("should update a deployment successfully with a certificate provided", async () => { - const { userApiKeySecret, wallets } = await mockUser(); - const dseq = "1234"; - setupDeploymentInfoMock(wallets, dseq); - - const mockTxResult = { - code: 0, - hash: "test-hash", - transactionHash: "test-hash", - rawLog: "success" - }; - - jest.spyOn(signerService, "executeDecodedTxByUserId").mockResolvedValueOnce(mockTxResult); - - const yml = fs.readFileSync(path.resolve(__dirname, "../mocks/hello-world-sdl.yml"), "utf8"); - - const response = await app.request(`/v1/deployments/${dseq}`, { - method: "PUT", - body: JSON.stringify({ - data: { - sdl: yml, - certificate: { - certPem: "test-cert-pem", - keyPem: "test-key-pem" - } - } - }), - headers: new Headers({ "Content-Type": "application/json", "x-api-key": userApiKeySecret }) - }); - - expect(response.status).toBe(200); - const result = (await response.json()) as { data: unknown }; - expect(result.data).toEqual({ - deployment: expect.any(Object), - escrow_account: expect.any(Object), - leases: expect.arrayContaining([expect.any(Object)]) - }); - });apps/api/src/provider/services/provider/provider.service.ts (1)
74-81: Preserve original error and broaden retry condition (handle provider HTTP errors)Short result of verification: rg returned no other occurrences of "no lease for deployment" and no import of provider-proxy in apps/api/src/provider/services/provider/provider.service.ts — the issue is local to this file and the suggested change is appropriate.
Please apply the fix at:
- apps/api/src/provider/services/provider/provider.service.ts — catch block (around lines 74–81)
Apply:
- } catch (err: any) { - if (err.message?.includes("no lease for deployment") && i < this.MANIFEST_SEND_MAX_RETRIES) { - await delay(this.MANIFEST_SEND_RETRY_DELAY); - continue; - } - throw new Error(err?.response?.data || err); - } + } catch (err) { + // Retry if the provider reports missing lease yet (eventual consistency) or 404 + const message = (err as any)?.message ?? ""; + const status = (err as any)?.response?.status as number | undefined; + if ((typeof message === "string" && message.includes("no lease for deployment")) || status === 404) { + if (i < this.MANIFEST_SEND_MAX_RETRIES) { + await delay(this.MANIFEST_SEND_RETRY_DELAY); + continue; + } + } + // Preserve original error to keep the stack and provider payload + throw err; + }Reason: string-matching err.message is brittle; providers commonly return HTTP errors (inspect response.status). Wrapping the original error in a new Error loses the provider payload and stack — rethrow the original error instead.
♻️ Duplicate comments (1)
apps/api/src/provider/services/provider/provider.service.ts (1)
11-12: Stop importing ProviderIdentity from deprecated provider-proxy; define a local type.We should avoid coupling to provider-proxy. The current import contradicts earlier review feedback and the migration away from provider-proxy.
Apply within this range:
-import { ProviderIdentity } from "@src/provider/services/provider/provider-proxy.service"; +// Using a local minimal identity type to avoid coupling to deprecated provider-proxyAdd this local type near the top of the file (e.g., after imports):
type ProviderIdentity = { owner: string; hostUri: string };
🧹 Nitpick comments (8)
packages/http-sdk/src/provider/provider-http.service.ts (2)
15-24: Consider adding error handling for manifest sending.The
sendManifestmethod should handle potential errors more gracefully, particularly for manifest validation or network issues.Consider wrapping the request in a try-catch to provide more context:
async sendManifest({ hostUri, dseq, manifest, jwtToken }: { hostUri: string; dseq: string; manifest: string; jwtToken: string }) { - return this.extractData( - await this.put(`/deployment/${dseq}/manifest`, { - baseURL: hostUri, - body: manifest, - headers: this.getJwtTokenHeaders(jwtToken), - timeout: 60000 - }) - ); + try { + return this.extractData( + await this.put(`/deployment/${dseq}/manifest`, { + baseURL: hostUri, + body: manifest, + headers: this.getJwtTokenHeaders(jwtToken), + timeout: 60000 + }) + ); + } catch (error) { + // Re-throw with more context for better debugging + throw new Error(`Failed to send manifest to ${hostUri}/deployment/${dseq}: ${error.message}`); + } }
36-41: Consider making JWT header generation more flexible.The current implementation hard-codes the
Content-Typeheader, which might not be appropriate for all endpoints (especially GET requests that don't have a body).Consider making the Content-Type header conditional:
-private getJwtTokenHeaders(jwtToken: string) { +private getJwtTokenHeaders(jwtToken: string, includeContentType = true) { + const headers: Record<string, string> = { + Authorization: `Bearer ${jwtToken}` + }; + + if (includeContentType) { + headers["Content-Type"] = "application/json"; + } + + return headers; +}Then update the
getLeaseStatuscall:headers: this.getJwtTokenHeaders(jwtToken, false),apps/api/examples/lease-flow.ts (1)
106-113: Consider making the provider selection configurable.The hard-coded provider address makes the example less flexible and may fail if that specific provider is unavailable.
Consider making the provider selection configurable via environment variable:
// Select a bid from a specific provider -const targetProvider = "akash175llqyjvxfle9qwt740vm46772dzaznpzgm576"; +const targetProvider = process.env.TARGET_PROVIDER || "akash175llqyjvxfle9qwt740vm46772dzaznpzgm576"; const selectedBid = bids.find(bid => bid.bid.bid_id.provider === targetProvider); if (!selectedBid) { - throw new Error(`No bid found from provider ${targetProvider}`); + // If specific provider not found, use the first available bid + console.warn(`No bid found from provider ${targetProvider}, using first available bid`); + const fallbackBid = bids[0]; + if (!fallbackBid) { + throw new Error("No bids available"); + } + return fallbackBid; }apps/api/src/provider/services/provider/provider.service.spec.ts (1)
83-84: Consider adding assertion for retry delay.The test verifies retry behavior but doesn't verify that there's an appropriate delay between retries to avoid overwhelming the provider.
Consider using jest's timer mocks to verify retry delays:
it("should retry on lease not found error with delay", async () => { jest.useFakeTimers(); const { service, jwtTokenService, providerHttpService } = setup(); // ... existing setup ... const resultPromise = service.sendManifest({ provider: providerAddress, dseq, manifest, walletId }); // Fast-forward time for first retry jest.advanceTimersByTime(2000); // or whatever the retry delay is const result = await resultPromise; expect(providerHttpService.sendManifest).toHaveBeenCalledTimes(2); expect(result).toEqual({ success: true }); jest.useRealTimers(); });apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (1)
42-59: Consider adding error logging for failed lease status retrievals.Silent failures in lease status retrieval might make debugging difficult in production.
Consider adding error logging:
} catch { + console.warn(`Failed to retrieve lease status for provider ${lease.lease_id.provider}, dseq ${lease.lease_id.dseq}`); return { lease, status: null }; }apps/api/src/provider/services/provider/provider.service.ts (3)
31-33: Avoid brittle string replacement for manifest mutation; perform a JSON-level transform.Direct regex replacement on a JSON string is error-prone and can break if formatting changes. Prefer parsing and transforming the structure, with a safe fallback if parsing fails.
Apply within this range:
- const manifestWithSize = manifest.replace(/"quantity":{"val/g, '"size":{"val'); + const manifestWithSize = transformManifestQuantitiesToSize(manifest);Add this helper in the file (outside this range):
function transformManifestQuantitiesToSize(manifestJson: string): string { try { const obj = JSON.parse(manifestJson); const walk = (node: any) => { if (node && typeof node === "object") { if ("quantity" in node && node.quantity && typeof node.quantity === "object" && "val" in node.quantity) { node.size = node.quantity; delete node.quantity; } for (const k of Object.keys(node)) walk((node as any)[k]); } }; walk(obj); return JSON.stringify(obj); } catch { // Fallback to legacy behavior if the manifest isn't strict JSON return manifestJson.replace(/"quantity":{"val/g, '"size":{"val'); } }
49-59: Consider consolidating provider identity resolution and caching.sendManifest and getLeaseStatus both call getProvider to resolve host_uri for the same provider address. Introduce a small cache (TTL) to avoid repeated network lookups under load.
If desired, I can add a memoized resolveProviderIdentity(address) that caches hostUri for a few minutes, similar to JwtTokenService.getJwtToken memoization.
84-99: Status path JWT scope and direct provider call — LGTM; consider provider identity caching.
- Using scope "status" is consistent with granular access for read-only status calls.
- Repeating getProvider lookups could be cached alongside sendManifest, as noted above.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snappackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
apps/api/examples/lease-flow.ts(9 hunks)apps/api/package.json(1 hunks)apps/api/src/billing/lib/wallet/wallet.ts(1 hunks)apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts(3 hunks)apps/api/src/deployment/http-schemas/deployment.schema.ts(1 hunks)apps/api/src/deployment/http-schemas/lease.schema.ts(0 hunks)apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts(5 hunks)apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts(3 hunks)apps/api/src/deployment/services/lease/lease.service.ts(1 hunks)apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts(1 hunks)apps/api/src/provider/services/jwt-token/jwt-token.service.ts(1 hunks)apps/api/src/provider/services/provider/provider.service.spec.ts(1 hunks)apps/api/src/provider/services/provider/provider.service.ts(3 hunks)apps/api/test/functional/deployments.spec.ts(7 hunks)apps/api/test/functional/lease-flow.spec.ts(3 hunks)packages/http-sdk/src/provider/provider-http.service.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/api/src/deployment/http-schemas/lease.schema.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/api/package.json
- apps/api/src/billing/lib/wallet/wallet.ts
- apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts
- apps/api/src/deployment/http-schemas/deployment.schema.ts
- apps/api/src/provider/services/jwt-token/jwt-token.service.ts
- apps/api/src/deployment/services/lease/lease.service.ts
- apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts
- apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
packages/http-sdk/src/provider/provider-http.service.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/api/src/provider/services/provider/provider.service.spec.tsapps/api/examples/lease-flow.tsapps/api/src/provider/services/provider/provider.service.tsapps/api/test/functional/deployments.spec.tsapps/api/test/functional/lease-flow.spec.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
packages/http-sdk/src/provider/provider-http.service.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/api/src/provider/services/provider/provider.service.spec.tsapps/api/examples/lease-flow.tsapps/api/src/provider/services/provider/provider.service.tsapps/api/test/functional/deployments.spec.tsapps/api/test/functional/lease-flow.spec.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/provider/services/provider/provider.service.spec.tsapps/api/test/functional/deployments.spec.tsapps/api/test/functional/lease-flow.spec.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: baktun14
PR: akash-network/console#1312
File: packages/jwt/src/jwt-token.ts:0-0
Timestamp: 2025-05-13T17:35:25.875Z
Learning: In the Akash JWT implementation, cryptographic signature verification is handled by the provider node code rather than in the JWT package itself. The JWT package only handles token creation, decoding, and basic validation (schema and time constraints).
📚 Learning: 2025-06-03T15:06:34.211Z
Learnt from: baktun14
PR: akash-network/console#1428
File: apps/api/src/deployment/controllers/deployment/deployment.controller.ts:0-0
Timestamp: 2025-06-03T15:06:34.211Z
Learning: The `getByOwnerAndDseq` method in `apps/api/src/deployment/controllers/deployment/deployment.controller.ts` is intentionally public without the `Protected` decorator because it serves public blockchain data from an indexer, following the pattern of public blockchain APIs.
Applied to files:
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts
📚 Learning: 2025-06-08T03:07:13.871Z
Learnt from: stalniy
PR: akash-network/console#1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Applied to files:
apps/api/src/provider/services/provider/provider.service.spec.ts
🧬 Code Graph Analysis (4)
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (2)
apps/api/src/deployment/http-schemas/deployment.schema.ts (1)
GetDeploymentResponse(248-248)apps/indexer/drizzle/schema.ts (2)
lease(110-178)deployment(180-203)
apps/api/src/provider/services/provider/provider.service.spec.ts (1)
packages/http-sdk/src/provider/provider-http.service.ts (1)
ProviderHttpService(6-42)
apps/api/src/provider/services/provider/provider.service.ts (6)
apps/api/src/deployment/services/lease/lease.service.ts (1)
singleton(10-38)apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts (1)
singleton(22-132)apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (1)
singleton(18-296)apps/provider-proxy/src/services/ProviderService/ProviderService.ts (1)
ProviderService(7-45)packages/http-sdk/src/provider/provider-http.service.ts (1)
ProviderHttpService(6-42)apps/api/src/deployment/http-schemas/lease.schema.ts (1)
LeaseStatusResponse(49-49)
apps/api/test/functional/deployments.spec.ts (1)
apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts (1)
UserWalletOutput(16-20)
⏰ 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). (12)
- GitHub Check: validate (apps/indexer) / validate-unsafe
- GitHub Check: test-build
- 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
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Validate local packages
🔇 Additional comments (17)
apps/api/examples/lease-flow.ts (4)
5-5: Documentation update looks good.The updated documentation correctly reflects the removal of certificate creation and the reliance on API key authentication.
81-100: Deployment creation step correctly updated.The deployment creation now properly uses only the SDL and deposit without any certificate data, aligning with the JWT authentication approach.
114-138: Lease creation correctly updated for JWT flow.The lease creation request properly includes the manifest and lease details without certificate data, which aligns with the JWT authentication model.
166-167: Update deployment correctly uses SDL only.The deployment update now correctly sends only the SDL without certificate data, consistent with the JWT-based approach.
apps/api/test/functional/lease-flow.spec.ts (2)
117-131: Excellent test parameterization for both certificate scenarios.The parameterized test approach effectively validates the lease lifecycle with and without certificates, ensuring backward compatibility while testing the new JWT flow.
196-201: Certificate handling is correctly conditional.The conditional certificate inclusion based on the
includeCertificateparameter properly tests both authentication paths.apps/api/src/provider/services/provider/provider.service.spec.ts (2)
225-240: Well-structured test setup using the setup pattern.The setup function correctly follows the coding guidelines by being at the bottom of the describe block, accepting inline type definitions, and not specifying a return type.
51-51: Incorrect — manifest transformation exists in code; test is validThe transformation is implemented in the service, so the test expectation is correct.
Files to note:
- apps/api/src/provider/services/provider/provider.service.ts —
const manifestWithSize = manifest.replace(/"quantity":{"val/g, '"size":{"val');- apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts —
JSON.stringify(manifest).replaceAll('"quantity":{"val', '"size":{"val');- packages/http-sdk/src/provider/provider-http.service.ts —
sendManifestsends the manifest string as the request bodyLikely an incorrect or invalid review comment.
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (2)
10-10: Good addition of UserWalletRepository dependency.The addition of UserWalletRepository to support wallet-based authentication aligns well with the JWT authentication approach.
Also applies to: 25-25
288-295: Wallet lookup error handling is properly implemented.The helper method correctly validates wallet existence and provides a clear error message when not found.
apps/api/test/functional/deployments.spec.ts (6)
43-51: Shared wallet cache looks good; reset prevents cross-test bleed.Defining allWallets and resetting it in beforeEach is a simple way to back the new findOneBy mock across tests. This avoids cross-test contamination.
117-118: Populate allWallets for cross-lookup — good call.Pushing wallets into the shared collection aligns the findOneBy mock with real behavior.
694-711: PUT without certificate path — LGTM.This validates the new contract (SDL-only) and the expected response shape. Mocks for sendManifest and getLeaseStatus keep the test isolated from provider I/O.
760-769: 404 path updated to SDL-only — LGTM.Matches the new contract and validates error handling when deployment is missing.
781-789: 401 unauthenticated path updated to SDL-only — LGTM.Consistent with JWT-only API; good coverage for auth failure behavior.
805-813: 400 invalid SDL path — LGTM.Keeps the negative case aligned with the simplified request body.
apps/api/src/provider/services/provider/provider.service.ts (1)
62-73: JWT scope usage looks correct.Granular leases with "send-manifest" scope is consistent with the new flow.
| const wallet = await this.getWalletByAddress(owner); | ||
| const leaseStatuses = await Promise.all( | ||
| leaseResults.map(async ({ leases }) => { | ||
| return await Promise.all( | ||
| leases.map(async ({ lease }) => { | ||
| return await this.providerService.getLeaseStatus(lease.lease_id.provider, lease.lease_id.dseq, lease.lease_id.gseq, lease.lease_id.oseq, wallet.id); | ||
| }) | ||
| ); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Performance concern: Nested Promise.all could impact response time.
The nested Promise.all structure for fetching lease statuses might create performance issues with many deployments and leases. Each deployment's leases are processed sequentially within the outer Promise.all.
Consider flattening the promise structure for better concurrency:
-const wallet = await this.getWalletByAddress(owner);
-const leaseStatuses = await Promise.all(
- leaseResults.map(async ({ leases }) => {
- return await Promise.all(
- leases.map(async ({ lease }) => {
- return await this.providerService.getLeaseStatus(lease.lease_id.provider, lease.lease_id.dseq, lease.lease_id.gseq, lease.lease_id.oseq, wallet.id);
- })
- );
- })
-);
+const wallet = await this.getWalletByAddress(owner);
+
+// Flatten all lease status requests into a single array
+const allLeaseStatusRequests = leaseResults.flatMap(({ leases }, deploymentIndex) =>
+ leases.map(({ lease }, leaseIndex) => ({
+ deploymentIndex,
+ leaseIndex,
+ request: this.providerService.getLeaseStatus(
+ lease.lease_id.provider,
+ lease.lease_id.dseq,
+ lease.lease_id.gseq,
+ lease.lease_id.oseq,
+ wallet.id
+ ).catch(() => null) // Handle errors gracefully
+ }))
+);
+
+// Execute all requests concurrently
+const statusResults = await Promise.all(
+ allLeaseStatusRequests.map(({ request }) => request)
+);
+
+// Reorganize results back into nested structure
+const leaseStatuses: any[][] = [];
+allLeaseStatusRequests.forEach((item, index) => {
+ if (!leaseStatuses[item.deploymentIndex]) {
+ leaseStatuses[item.deploymentIndex] = [];
+ }
+ leaseStatuses[item.deploymentIndex][item.leaseIndex] = statusResults[index];
+});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts
around lines 86 to 95, the nested Promise.all causes sub-optimal concurrency
because each outer map waits for its inner Promise.all; instead flatten all
lease lookups into a single array of providerService.getLeaseStatus calls and
await them once. Collect all lease entries (with their lease_id parts and
wallet.id) into a single array via a single pass over leaseResults, map that
array to calls to this.providerService.getLeaseStatus without awaiting inside
the map, then await Promise.all on that flattened array so all requests run
concurrently.
| @@ -0,0 +1,241 @@ | |||
| import type { ProviderHttpService } from "@akashnetwork/http-sdk"; | |||
| import type { JwtTokenPayload } from "@akashnetwork/jwt/src/types"; | |||
There was a problem hiding this comment.
Fix incorrect import path for JwtTokenPayload type.
The import path @akashnetwork/jwt/src/types appears to be reaching into the source directory directly. This should import from the package root.
-import type { JwtTokenPayload } from "@akashnetwork/jwt/src/types";
+import type { JwtTokenPayload } from "@akashnetwork/jwt";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type { JwtTokenPayload } from "@akashnetwork/jwt/src/types"; | |
| import type { JwtTokenPayload } from "@akashnetwork/jwt"; |
🤖 Prompt for AI Agents
In apps/api/src/provider/services/provider/provider.service.spec.ts around line
2, the test imports JwtTokenPayload from the package's internal src path; change
the import to come from the package root (e.g. import type { JwtTokenPayload }
from "@akashnetwork/jwt") so it uses the public package export surface rather
than reaching into the library's src directory.
| jest.spyOn(userWalletRepository, "findOneBy").mockImplementation(async (query: Partial<UserWalletOutput> | undefined) => { | ||
| return Promise.resolve(allWallets.find(wallet => wallet.address === query?.address)); | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor spec to use setup() instead of beforeEach; simplify async mock.
- This spec still uses beforeEach while our spec guidelines require a setup function created inside the root describe. Please refactor the spec to follow our testing conventions.
- Minor: since the implementation is already async, returning Promise.resolve is redundant; just return the found wallet.
Apply within this range:
- jest.spyOn(userWalletRepository, "findOneBy").mockImplementation(async (query: Partial<UserWalletOutput> | undefined) => {
- return Promise.resolve(allWallets.find(wallet => wallet.address === query?.address));
- });
+ jest.spyOn(userWalletRepository, "findOneBy").mockImplementation(async (query: Partial<UserWalletOutput> | undefined) => {
+ return allWallets.find(wallet => wallet.address === query?.address);
+ });If you want, I can provide a full patch converting this suite to a setup() pattern per our test guidelines.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jest.spyOn(userWalletRepository, "findOneBy").mockImplementation(async (query: Partial<UserWalletOutput> | undefined) => { | |
| return Promise.resolve(allWallets.find(wallet => wallet.address === query?.address)); | |
| }); | |
| jest.spyOn(userWalletRepository, "findOneBy").mockImplementation(async (query: Partial<UserWalletOutput> | undefined) => { | |
| return allWallets.find(wallet => wallet.address === query?.address); | |
| }); |
🤖 Prompt for AI Agents
In apps/api/test/functional/deployments.spec.ts around lines 71-74, refactor the
test to use a setup() function declared in the root describe instead of
beforeEach, and simplify the mock by removing the redundant Promise.resolve;
replace the current jest.spyOn(...).mockImplementation(async (query) => { return
Promise.resolve(allWallets.find(...)); }) with a mockImplementation that
directly returns the found wallet (e.g., async (query) => allWallets.find(wallet
=> wallet.address === query?.address)); place this spy inside the new setup()
and invoke setup() from tests per the repo's testing conventions.
jzsfkzm
force-pushed
the
features/1038-jwt-not-cert
branch
from
3f93180 to
167b68f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
apps/api/test/functional/lease-flow.spec.ts (1)
263-263: Deployment update always includes certificate — test doesn't exercise the certificate-absent caseThe PUT to /v1/deployments in apps/api/test/functional/lease-flow.spec.ts includes certificate unconditionally, so the "without a certificate provided" test case never exercises a certificate-free update.
Files / locations to change
- apps/api/test/functional/lease-flow.spec.ts — around lines 261–263 (the PUT /v1/deployments update call).
- (Reference) The lease creation uses a conditional certificate already (lines ~198–205); the update should mirror that.
Suggested change (replace the existing line)
Original:body: JSON.stringify({ data: { sdl: ymlUpdate, certificate: { certPem, keyPem: encryptedKey } } })Replace with:
body: JSON.stringify({ data: { sdl: ymlUpdate, certificate: includeCertificate ? { certPem, keyPem: encryptedKey } : undefined } })Please apply the change so the test covers both certificate-present and certificate-absent update flows.
apps/api/src/provider/services/provider/provider.service.ts (1)
60-82: Ensure deterministic return after retries; preserve original error context.Two improvements:
- If sendManifest returns a falsy value without throwing, the function exits without returning, yielding undefined. Return false (or throw) after exhausting retries to keep return type stable.
- Wrapping the original error with new Error(...) loses stack and can stringify objects to “[object Object]”. Prefer throwing the original error or a stringified response safely.
} catch (err: any) { if (err.message?.includes("no lease for deployment") && i < this.MANIFEST_SEND_MAX_RETRIES) { await delay(this.MANIFEST_SEND_RETRY_DELAY); continue; } - throw new Error(err?.response?.data || err); + if (err?.response?.data) { + const data = typeof err.response.data === "string" ? err.response.data : JSON.stringify(err.response.data); + throw new Error(data); + } + // Preserve original error and stack if possible + throw err instanceof Error ? err : new Error(String(err)); } } + // All retries exhausted; return a consistent boolean or consider throwing + return false;
♻️ Duplicate comments (3)
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (1)
86-95: Nested Promise.all structure impacts performance.The nested Promise.all creates suboptimal concurrency as each outer map waits for its inner Promise.all to complete. This was flagged in previous reviews and remains unaddressed.
apps/api/test/functional/deployments.spec.ts (1)
71-73: Simplify async mock; drop redundant Promise.resolve and adopt setup() per guidelines.You don’t need Promise.resolve inside an async mock. Also, as previously requested, move this into a setup() function instead of beforeEach.
Apply this diff:
- jest.spyOn(userWalletRepository, "findOneBy").mockImplementation(async (query: Partial<UserWalletOutput> | undefined) => { - return Promise.resolve(allWallets.find(wallet => wallet.address === query?.address)); - }); + jest.spyOn(userWalletRepository, "findOneBy").mockImplementation(async (query: Partial<UserWalletOutput> | undefined) => { + return allWallets.find(wallet => wallet.address === query?.address); + });apps/api/src/provider/services/provider/provider.service.ts (1)
11-12: Decouple from deprecated provider-proxy type.Define a minimal local type instead of importing ProviderIdentity from a deprecated path.
-import { ProviderIdentity } from "@src/provider/services/provider/provider-proxy.service"; +type ProviderIdentity = { owner: string; hostUri: string };
🧹 Nitpick comments (6)
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts (1)
19-19: Consider using consistent mock naming.The new
findWalletBymock follows a different naming pattern than the existing mocks. Consider aligning with the existing pattern for better consistency.-const findWalletBy = jest.fn().mockImplementation(async () => newWallet); +const findOneBy = jest.fn().mockImplementation(async () => newWallet);And update the setup call:
const di = setup({ - findWalletBy, + findOneBy, findWalletByUserId, createWallet, createAndAuthorizeTrialSpending, updateWalletById });Also applies to: 25-26
apps/api/examples/lease-flow.ts (1)
106-124: Consider making provider selection more flexible.The hardcoded provider selection might make the example less useful for different environments. Consider adding this as an environment variable or selecting the first available provider.
// Select a bid from a specific provider -const targetProvider = "akash175llqyjvxfle9qwt740vm46772dzaznpzgm576"; -const selectedBid = bids.find(bid => bid.bid.bid_id.provider === targetProvider); +const targetProvider = process.env.TARGET_PROVIDER || bids[0]?.bid?.bid_id?.provider; +const selectedBid = bids.find(bid => bid.bid.bid_id.provider === targetProvider) || bids[0]; if (!selectedBid) { - throw new Error(`No bid found from provider ${targetProvider}`); + throw new Error(`No bids available`); }apps/api/test/functional/deployments.spec.ts (1)
43-51: Avoid shared mutable state; prefer setup() pattern per test guidelines.The allWallets array introduces cross-test shared state (even though reset in beforeEach). Per our spec guidelines, move this into a setup() function scoped to each test and avoid sharing state across tests.
If you want, I can provide a targeted patch converting this suite to a setup() pattern and removing the shared array.
Also applies to: 117-118
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts (2)
2-2: Avoid deep import from package internals.Importing types from @akashnetwork/jwt/src/types couples tests to internal paths. Prefer the package root export for long-term stability.
Proposed change:
-import type { JwtTokenPayload } from "@akashnetwork/jwt/src/types"; +import type { JwtTokenPayload } from "@akashnetwork/jwt";If JwtTokenPayload isn’t exported at the package root yet, consider re-exporting it there to avoid deep imports.
23-31: Restore Date.now mock to prevent bleed across tests.The Date.now spy isn’t restored and can leak into other tests. Restore it at the end of the test.
- const now = Math.floor(Date.now() / 1000); - jest.spyOn(Date, "now").mockReturnValue(now * 1000); + const now = Math.floor(Date.now() / 1000); + const nowSpy = jest.spyOn(Date, "now").mockReturnValue(now * 1000); const result = JSON.parse(await jwtTokenService.generateJwtToken({ walletId: mockWalletId, leases })); + nowSpy.mockRestore();apps/api/src/provider/services/provider/provider.service.ts (1)
62-69: Consider token reuse to minimize signing overhead.If generateJwtToken signs on every call, cache tokens keyed by {walletId, provider, scope} until exp minus a small skew (e.g., 2–5 seconds) to reduce overhead, per earlier discussion. The JwtTokenService may already memoize; ensure reuse across ProviderService calls.
Would you like me to add a small LRU or time-based cache wrapper here keyed by walletId+provider+scope?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snappackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
apps/api/examples/lease-flow.ts(9 hunks)apps/api/package.json(1 hunks)apps/api/src/billing/lib/wallet/wallet.ts(1 hunks)apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts(3 hunks)apps/api/src/deployment/http-schemas/deployment.schema.ts(1 hunks)apps/api/src/deployment/http-schemas/lease.schema.ts(0 hunks)apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts(5 hunks)apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts(3 hunks)apps/api/src/deployment/services/lease/lease.service.ts(1 hunks)apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts(1 hunks)apps/api/src/provider/services/jwt-token/jwt-token.service.ts(1 hunks)apps/api/src/provider/services/provider/provider.service.spec.ts(1 hunks)apps/api/src/provider/services/provider/provider.service.ts(3 hunks)apps/api/test/functional/deployments.spec.ts(7 hunks)apps/api/test/functional/lease-flow.spec.ts(3 hunks)packages/http-sdk/src/provider/provider-http.service.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/api/src/deployment/http-schemas/lease.schema.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/api/package.json
- apps/api/src/billing/lib/wallet/wallet.ts
- apps/api/src/deployment/services/lease/lease.service.ts
- apps/api/src/provider/services/provider/provider.service.spec.ts
- apps/api/src/deployment/http-schemas/deployment.schema.ts
- apps/api/src/provider/services/jwt-token/jwt-token.service.ts
- packages/http-sdk/src/provider/provider-http.service.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/test/functional/deployments.spec.tsapps/api/src/provider/services/jwt-token/jwt-token.service.spec.tsapps/api/test/functional/lease-flow.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/api/test/functional/deployments.spec.tsapps/api/src/provider/services/jwt-token/jwt-token.service.spec.tsapps/api/src/deployment/services/deployment-writer/deployment-writer.service.tsapps/api/examples/lease-flow.tsapps/api/src/provider/services/provider/provider.service.tsapps/api/test/functional/lease-flow.spec.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/deployment/services/deployment-reader/deployment-reader.service.tsapps/api/test/functional/deployments.spec.tsapps/api/src/provider/services/jwt-token/jwt-token.service.spec.tsapps/api/src/deployment/services/deployment-writer/deployment-writer.service.tsapps/api/examples/lease-flow.tsapps/api/src/provider/services/provider/provider.service.tsapps/api/test/functional/lease-flow.spec.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: baktun14
PR: akash-network/console#1312
File: packages/jwt/src/jwt-token.ts:0-0
Timestamp: 2025-05-13T17:35:25.875Z
Learning: In the Akash JWT implementation, cryptographic signature verification is handled by the provider node code rather than in the JWT package itself. The JWT package only handles token creation, decoding, and basic validation (schema and time constraints).
📚 Learning: 2025-06-03T15:06:34.211Z
Learnt from: baktun14
PR: akash-network/console#1428
File: apps/api/src/deployment/controllers/deployment/deployment.controller.ts:0-0
Timestamp: 2025-06-03T15:06:34.211Z
Learning: The `getByOwnerAndDseq` method in `apps/api/src/deployment/controllers/deployment/deployment.controller.ts` is intentionally public without the `Protected` decorator because it serves public blockchain data from an indexer, following the pattern of public blockchain APIs.
Applied to files:
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts
📚 Learning: 2025-07-21T08:24:27.953Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-07-21T08:24:27.953Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Applied to files:
apps/api/test/functional/deployments.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Applied to files:
apps/api/test/functional/deployments.spec.ts
📚 Learning: 2025-07-21T08:24:24.269Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test.
Applied to files:
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts
🧬 Code Graph Analysis (4)
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (2)
apps/api/src/deployment/http-schemas/deployment.schema.ts (1)
GetDeploymentResponse(248-248)apps/indexer/drizzle/schema.ts (2)
lease(110-178)deployment(180-203)
apps/api/test/functional/deployments.spec.ts (1)
apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts (1)
UserWalletOutput(16-20)
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts (2)
apps/api/src/billing/providers/config.provider.ts (1)
BillingConfig(9-9)apps/api/src/billing/lib/wallet/wallet.ts (1)
Wallet(7-53)
apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts (2)
apps/indexer/drizzle/schema.ts (3)
deployment(180-203)lease(110-178)provider(205-237)apps/api/src/deployment/http-schemas/deployment.schema.ts (1)
GetDeploymentResponse(248-248)
⏰ 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). (15)
- GitHub Check: validate (apps/provider-console) / 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: 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: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (28)
apps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.ts (4)
5-5: LGTM! JWT service dependency added correctly.The import of JwtTokenService aligns with the PR's JWT authentication implementation.
98-112: Improved mock structure using jest-mock-extended.The refactored UserWalletRepository mock using
jest-mock-extendedprovides better type safety and cleaner mock definition compared to inline mocks.
119-124: JWT service mock properly configured.The JwtTokenService mock with predictable "mock-jwt-token" return value is appropriate for testing the wallet initializer in isolation.
132-132: Interface extension follows the pattern correctly.The SetupInput interface extension with optional
findWalletByproperty maintains consistency with the existing interface design.apps/api/examples/lease-flow.ts (4)
1-20: Documentation updated correctly to reflect JWT authentication.The comments properly document the shift from certificate-based to API key authentication, with clear requirements and updated step numbering.
71-79: Proper API key validation added upfront.The early validation ensures the script fails fast if the required API_KEY environment variable is missing.
126-137: Proper error handling for lease creation.The status check and error throwing for non-200 responses ensures the script fails appropriately when lease creation fails.
166-166: Certificate data correctly removed from update payload.The update payload now only includes SDL data, consistent with the JWT authentication approach.
apps/api/src/deployment/services/deployment-reader/deployment-reader.service.ts (4)
10-10: UserWalletRepository integration added correctly.The dependency injection of UserWalletRepository enables wallet-based lease status retrieval, supporting the JWT authentication flow.
Also applies to: 24-25
28-29: Method signature properly updated for JWT approach.The removal of the optional certificate parameter and return type specification aligns with the shift to wallet-based authentication.
40-61: Error handling added for lease status retrieval.The try-catch wrapper around
providerService.getLeaseStatusgracefully handles potential errors by setting status to null, preventing the entire operation from failing due to provider unavailability.
288-295: Wallet lookup implementation with proper error handling.The
getWalletByAddresshelper method correctly handles the case where no wallet is found for the given address, throwing an appropriate error message.apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts (4)
95-95: Certificate handling correctly removed from update flow.The destructuring now only extracts
sdlfrom the input, removing certificate dependency as expected for JWT authentication.
104-104: Method signature updated to use wallet ID.The
sendManifestToProviderscall now useswallet.idas the first parameter, replacing certificate-based authentication with wallet-based JWT authentication.
106-106: Certificate parameter removed from deployment lookup.The final call to
deploymentReaderService.findByOwnerAndDseqcorrectly omits the certificate parameter, consistent with the new JWT approach.
126-131: Provider manifest sending updated to JWT approach.The method signature change to use
walletIdinstead of certificate and the updatedproviderService.sendManifestcall structure align with the JWT authentication implementation.apps/api/test/functional/lease-flow.spec.ts (3)
118-131: Parameterized tests for certificate scenarios.The parameterized test approach effectively covers both certificate-present and certificate-absent scenarios, ensuring backward compatibility while testing the new JWT flow.
133-297: Well-structured lifecycle test function.The
runLifecyclefunction properly encapsulates the complete lease flow with conditional certificate handling, maintaining all existing test assertions while adding flexibility for testing both scenarios.
200-205: Certificate handling correctly conditional.The conditional certificate inclusion in the lease creation payload based on the
includeCertificateparameter allows testing both the legacy certificate path and the new JWT path.apps/api/test/functional/deployments.spec.ts (5)
694-711: LGTM: SDL-only update test reflects cert removal.Good coverage for the SDL-only update path, consistent with moving to wallet/JWT-based flows.
713-727: Confirm behavior when extra “certificate” field is provided.This test implies the endpoint ignores the certificate payload. Ensure the schema/router allows unknown fields (or explicitly ignores them) so we don't inadvertently accept unused sensitive inputs.
If unknown fields are rejected by the validator, this test could become flaky. Consider adding an explicit assertion (e.g., spy on providerService.sendManifest args) to verify the certificate is not used downstream.
760-766: LGTM: 404 path updated to SDL-only payload.Matches the certificate removal and keeps negative path assertions intact.
784-786: LGTM: 401 path updated to SDL-only payload.Consistent with new request shape.
808-810: LGTM: invalid SDL path updated.Retains validation behavior post-cert removal.
apps/api/src/provider/services/jwt-token/jwt-token.service.spec.ts (1)
61-72: LGTM: unique jti coverage.Good assertion that each token gets a unique identifier even when generated in quick succession.
apps/api/src/provider/services/provider/provider.service.ts (3)
28-29: LGTM: JwtTokenService injection aligns with JWT migration.Constructor now directly depends on JwtTokenService, which is consistent with removing provider-proxy and certificate flows.
49-59: LGTM: internal sendManifest path uses JWT-scoped lease.Clear separation of concerns: identity lookup at the public method and token-scoped delivery inside the private method.
90-99: LGTM: status path uses provider-scoped JWT lease.The scope "status" and hostUri routing look correct for direct provider calls.
closes #1038
Summary by CodeRabbit
New Features
Improvements
Tests
Chores