Feat/vault contract function calls endpoints#58
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (60)
📒 Files selected for processing (22)
📝 WalkthroughWalkthroughThis PR adds a new Vault module (controller, service, module, DTOs), registers VaultModule in AppModule, extends SorobanService with readContractState for read-only contract calls, and includes miscellaneous formatting/newline adjustments across several core files and .gitignore. Changes
Sequence DiagramsequenceDiagram
participant Client
participant VaultController
participant VaultService
participant SorobanService
participant StellarContract
rect rgba(200,150,255,0.5)
Note over Client,StellarContract: Write operation (POST /vault/availability-for-exchange)
Client->>VaultController: POST AvailabilityForExchangeDto
VaultController->>VaultService: availabilityForExchange(dto)
VaultService->>SorobanService: buildContractCallTransaction(contractId, "availability_for_exchange", {admin, enabled}, callerPublicKey)
SorobanService->>StellarContract: construct unsigned transaction (simulate)
SorobanService-->>VaultService: unsignedXdr
VaultService-->>VaultController: { unsignedXdr }
VaultController-->>Client: { unsignedXdr }
end
rect rgba(150,200,255,0.5)
Note over Client,StellarContract: Read operation (GET /vault/overview)
Client->>VaultController: GET ?contractId=...&callerPublicKey=...
VaultController->>VaultService: getOverview(contractId, callerPublicKey)
VaultService->>SorobanService: readContractState(contractId, "get_vault_overview", {}, callerPublicKey)
SorobanService->>StellarContract: simulate read-only call
StellarContract-->>SorobanService: result
SorobanService-->>VaultService: result data
VaultService-->>VaultController: result data
VaultController-->>Client: VaultOverview JSON
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/core/src/vault/vault.service.ts (1)
9-18: Add a focused unit test for this Soroban call.
availability_for_exchangeand its argument keys are all inline literals here, so a typo will only surface at runtime. A small service spec that asserts the exactbuildContractCallTransaction(...)arguments would make this integration safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/core/src/vault/vault.service.ts` around lines 9 - 18, Add a focused unit test for VaultService.availabilityForExchange that spies/mocks SorobanService.buildContractCallTransaction and asserts it was called exactly once with the expected literal contract function name "availability_for_exchange", the exact argument object { admin: dto.admin, enabled: dto.enabled }, dto.contractId and dto.callerPublicKey in the correct parameter order; instantiate VaultService with a test double for soroban, call availabilityForExchange with a sample AvailabilityForExchangeDto, and assert the mock was invoked with those exact values (and that the method returns/forwards the mock's Promise result).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/core/src/deploy/deploy.service.ts`:
- Around line 13-20: The constructor in DeployService reads
PARTICIPATION_TOKEN_WASM_HASH, TOKEN_FACTORY_WASM_HASH and VAULT_WASM_HASH with
non-null assertions which defers failure to runtime; change this to validate
those env vars at startup: either read and throw a NestJS exception (e.g.,
InternalServerErrorException) from the DeployService constructor if any of
participationTokenWasmHash, tokenFactoryWasmHash or vaultWasmHash are missing,
or better, migrate these settings to `@nestjs/config` with a Joi/zod schema so
missing values are rejected during bootstrap; reference the DeployService
constructor and the private fields participationTokenWasmHash,
tokenFactoryWasmHash and vaultWasmHash when implementing the fix.
In `@apps/core/src/deploy/dto/deploy-vault.dto.ts`:
- Around line 1-26: Update the DeployVaultDto (class DeployVaultDto) to include
class-transformer decorators: add `@Type`(() => Number) to roiPercentage to coerce
numeric strings to numbers and add appropriate string transformation decorators
(e.g., trim/normalize via `@Transform`) to admin, token, usdc, callerPublicKey and
to enabled if needed for boolean coercion so incoming string values are
normalized/converted before validation; then enable transformation globally by
updating the ValidationPipe instantiation (ValidationPipe in main.ts) to include
transform: true (e.g., { whitelist: true, transform: true }) so the
class-transformer decorators are executed during request validation.
In `@apps/core/src/soroban/soroban.service.ts`:
- Line 43: Validate that the requested contract method exists and is callable
before invoking dynamic calls like client[method](args) (applies to both
occurrences where client[method](args) is used); resolve the property from
client, check typeof resolved === 'function', and if not throw a NestJS
BadRequestException (or HttpException) with a clear message including the
supplied method name, otherwise call the function safely (e.g., const fn =
(client as any)[method]; return await fn(args)). Ensure you reference the
variables client and method and replace unguarded dynamic invocation sites with
this guarded pattern so typos or unsupported method names produce a proper HTTP
error instead of an unhandled runtime exception.
---
Nitpick comments:
In `@apps/core/src/vault/vault.service.ts`:
- Around line 9-18: Add a focused unit test for
VaultService.availabilityForExchange that spies/mocks
SorobanService.buildContractCallTransaction and asserts it was called exactly
once with the expected literal contract function name
"availability_for_exchange", the exact argument object { admin: dto.admin,
enabled: dto.enabled }, dto.contractId and dto.callerPublicKey in the correct
parameter order; instantiate VaultService with a test double for soroban, call
availabilityForExchange with a sample AvailabilityForExchangeDto, and assert the
mock was invoked with those exact values (and that the method returns/forwards
the mock's Promise result).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c795bb7e-00aa-44f0-9239-380f267a59c7
⛔ Files ignored due to path filters (15)
apps/core/dist/app.module.jsis excluded by!**/dist/**apps/core/dist/app.module.js.mapis excluded by!**/dist/**,!**/*.mapapps/core/dist/campaigns/campaigns.controller.d.tsis excluded by!**/dist/**apps/core/dist/campaigns/campaigns.service.d.tsis excluded by!**/dist/**apps/core/dist/deploy/deploy.controller.d.tsis excluded by!**/dist/**apps/core/dist/deploy/deploy.controller.jsis excluded by!**/dist/**apps/core/dist/deploy/deploy.controller.js.mapis excluded by!**/dist/**,!**/*.mapapps/core/dist/deploy/deploy.service.d.tsis excluded by!**/dist/**apps/core/dist/deploy/deploy.service.jsis excluded by!**/dist/**apps/core/dist/deploy/deploy.service.js.mapis excluded by!**/dist/**,!**/*.mapapps/core/dist/deploy/dto/deploy-vault.dto.d.tsis excluded by!**/dist/**apps/core/dist/deploy/dto/deploy-vault.dto.jsis excluded by!**/dist/**apps/core/dist/deploy/dto/deploy-vault.dto.js.mapis excluded by!**/dist/**,!**/*.mapapps/core/dist/investments/investments.controller.d.tsis excluded by!**/dist/**apps/core/dist/investments/investments.service.d.tsis excluded by!**/dist/**
📒 Files selected for processing (23)
apps/core/README.mdapps/core/package.jsonapps/core/src/app.module.tsapps/core/src/campaigns/campaigns.controller.tsapps/core/src/campaigns/campaigns.module.tsapps/core/src/campaigns/campaigns.service.tsapps/core/src/campaigns/dto/update-campaign.dto.tsapps/core/src/deploy/deploy.controller.tsapps/core/src/deploy/deploy.service.tsapps/core/src/deploy/dto/deploy-participation-token.dto.tsapps/core/src/deploy/dto/deploy-vault.dto.tsapps/core/src/investments/dto/create-investment.dto.tsapps/core/src/investments/investments.controller.tsapps/core/src/investments/investments.module.tsapps/core/src/investments/investments.service.tsapps/core/src/loans/loans.service.spec.tsapps/core/src/prisma/prisma.module.tsapps/core/src/soroban/soroban.module.tsapps/core/src/soroban/soroban.service.tsapps/core/src/vault/dto/availability-for-exchange.dto.tsapps/core/src/vault/vault.controller.tsapps/core/src/vault/vault.module.tsapps/core/src/vault/vault.service.ts
| private readonly vaultWasmHash: string; | ||
|
|
||
| constructor(private readonly soroban: SorobanService) { | ||
| this.participationTokenWasmHash = | ||
| process.env.PARTICIPATION_TOKEN_WASM_HASH!; | ||
| this.tokenFactoryWasmHash = process.env.TOKEN_FACTORY_WASM_HASH!; | ||
| this.vaultWasmHash = process.env.VAULT_WASM_HASH!; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "VAULT_WASM_HASH|TOKEN_FACTORY_WASM_HASH|PARTICIPATION_TOKEN_WASM_HASH" apps/core/src apps/core/.env.example
rg -n "ConfigModule|ConfigService|validationSchema|Joi|zod|registerAs" apps/core/src apps/coreRepository: Trustless-Work/tokenization-private-credit
Length of output: 542
🏁 Script executed:
# Check how this service currently handles errors
head -50 apps/core/src/deploy/deploy.service.ts | tail -40
# Check for NestJS exception imports in this service
rg -n "BadRequestException|InternalServerErrorException|NotFoundException|throw" apps/core/src/deploy/deploy.service.ts | head -20
# Check app bootstrap for any env validation
rg -n "process.env|ConfigModule|validate" apps/core/src/main.ts apps/core/src/app.module.ts 2>/dev/null || trueRepository: Trustless-Work/tokenization-private-credit
Length of output: 1435
🏁 Script executed:
# Check if app.module.ts exists and has ConfigModule
cat apps/core/src/app.module.ts 2>/dev/null || echo "File not found"
# Check entire main.ts for bootstrap logic
cat apps/core/src/main.ts
# Search for any error class/exception definitions in the codebase
rg -n "class.*Error|class.*Exception" apps/core/src --max-count=10Repository: Trustless-Work/tokenization-private-credit
Length of output: 1216
Validate VAULT_WASM_HASH and other required env vars at startup.
Lines 17–19 read three WASM hash env vars with non-null assertions. If any are missing, the app boots successfully and fails only on first request with an unclear error. This pattern is particularly risky for configuration that is essential to deployment functionality.
Per coding guidelines, NestJS services should use built-in exceptions (e.g., BadRequestException, InternalServerErrorException) for error handling. Two options:
- Immediate validation in constructor: Throw a NestJS exception if
VAULT_WASM_HASH(and ideally the other two hashes) is missing. - ConfigModule approach (recommended): Use
@nestjs/configwith a validation schema (Joi/zod) to enforce required env vars at bootstrap time, consistent with NestJS best practices.
The codebase currently has no centralized env validation, and PORT already shows an inconsistent pattern with a fallback while WASM hashes rely on assertions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/core/src/deploy/deploy.service.ts` around lines 13 - 20, The constructor
in DeployService reads PARTICIPATION_TOKEN_WASM_HASH, TOKEN_FACTORY_WASM_HASH
and VAULT_WASM_HASH with non-null assertions which defers failure to runtime;
change this to validate those env vars at startup: either read and throw a
NestJS exception (e.g., InternalServerErrorException) from the DeployService
constructor if any of participationTokenWasmHash, tokenFactoryWasmHash or
vaultWasmHash are missing, or better, migrate these settings to `@nestjs/config`
with a Joi/zod schema so missing values are rejected during bootstrap; reference
the DeployService constructor and the private fields participationTokenWasmHash,
tokenFactoryWasmHash and vaultWasmHash when implementing the fix.
| import { IsString, IsBoolean, IsNumber, IsNotEmpty } from 'class-validator'; | ||
|
|
||
| export class DeployVaultDto { | ||
| @IsString() | ||
| @IsNotEmpty() | ||
| admin: string; | ||
|
|
||
| @IsBoolean() | ||
| @IsNotEmpty() | ||
| enabled: boolean; | ||
|
|
||
| @IsNumber() | ||
| @IsNotEmpty() | ||
| roiPercentage: number; | ||
|
|
||
| @IsString() | ||
| @IsNotEmpty() | ||
| token: string; | ||
|
|
||
| @IsString() | ||
| @IsNotEmpty() | ||
| usdc: string; | ||
|
|
||
| @IsString() | ||
| @IsNotEmpty() | ||
| callerPublicKey: string; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "ValidationPipe|transform:\s*true|enableImplicitConversion" apps/core/src/main.ts apps/core/src
rg -n "class-transformer|@Transform\(|@Type\(" apps/core/src --glob "*.dto.ts"Repository: Trustless-Work/tokenization-private-credit
Length of output: 405
🏁 Script executed:
cat apps/core/package.json | grep -A 5 -B 5 "class-transformer"Repository: Trustless-Work/tokenization-private-credit
Length of output: 418
🏁 Script executed:
fd "\.dto\.ts$" apps/core/src --exec head -20 {} \; -printRepository: Trustless-Work/tokenization-private-credit
Length of output: 316
🏁 Script executed:
cat apps/core/src/main.tsRepository: Trustless-Work/tokenization-private-credit
Length of output: 425
🏁 Script executed:
find apps/core/src -name "*.dto.ts" -type fRepository: Trustless-Work/tokenization-private-credit
Length of output: 600
🏁 Script executed:
cat apps/core/src/deploy/dto/deploy-vault.dto.tsRepository: Trustless-Work/tokenization-private-credit
Length of output: 503
🏁 Script executed:
cat apps/core/src/campaigns/dto/create-campaign.dto.tsRepository: Trustless-Work/tokenization-private-credit
Length of output: 645
🏁 Script executed:
cat apps/core/src/investments/dto/create-investment.dto.tsRepository: Trustless-Work/tokenization-private-credit
Length of output: 467
🏁 Script executed:
cat apps/core/src/loans/dto/create-loan.dto.tsRepository: Trustless-Work/tokenization-private-credit
Length of output: 482
🏁 Script executed:
rg "@Transform|@Type" apps/core/src --glob "*.dto.ts"Repository: Trustless-Work/tokenization-private-credit
Length of output: 68
Add class-transformer decorators to this DTO and enable transformation in ValidationPipe.
This DTO uses only class-validator decorators but lacks class-transformer per the coding guideline. Additionally, ValidationPipe in apps/core/src/main.ts is configured without transform: true, so even with decorators added, transformation won't execute. Both are required:
- Add
@Type(() => Number)toroiPercentagefor automatic string-to-number coercion - Add transformation decorators for string normalization where appropriate
- Update
ValidationPipeinapps/core/src/main.ts:7to include{ whitelist: true, transform: true }
This pattern applies across all DTOs in the project (10 total files found).
Minimal example for this DTO
+import { Transform, Type } from 'class-transformer';
import { IsString, IsBoolean, IsNumber, IsNotEmpty } from 'class-validator';
export class DeployVaultDto {
`@IsString`()
`@IsNotEmpty`()
admin: string;
`@IsBoolean`()
`@IsNotEmpty`()
enabled: boolean;
+ `@Type`(() => Number)
`@IsNumber`()
`@IsNotEmpty`()
roiPercentage: number;📝 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 { IsString, IsBoolean, IsNumber, IsNotEmpty } from 'class-validator'; | |
| export class DeployVaultDto { | |
| @IsString() | |
| @IsNotEmpty() | |
| admin: string; | |
| @IsBoolean() | |
| @IsNotEmpty() | |
| enabled: boolean; | |
| @IsNumber() | |
| @IsNotEmpty() | |
| roiPercentage: number; | |
| @IsString() | |
| @IsNotEmpty() | |
| token: string; | |
| @IsString() | |
| @IsNotEmpty() | |
| usdc: string; | |
| @IsString() | |
| @IsNotEmpty() | |
| callerPublicKey: string; | |
| import { Transform, Type } from 'class-transformer'; | |
| import { IsString, IsBoolean, IsNumber, IsNotEmpty } from 'class-validator'; | |
| export class DeployVaultDto { | |
| `@IsString`() | |
| `@IsNotEmpty`() | |
| admin: string; | |
| `@IsBoolean`() | |
| `@IsNotEmpty`() | |
| enabled: boolean; | |
| `@Type`(() => Number) | |
| `@IsNumber`() | |
| `@IsNotEmpty`() | |
| roiPercentage: number; | |
| `@IsString`() | |
| `@IsNotEmpty`() | |
| token: string; | |
| `@IsString`() | |
| `@IsNotEmpty`() | |
| usdc: string; | |
| `@IsString`() | |
| `@IsNotEmpty`() | |
| callerPublicKey: string; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/core/src/deploy/dto/deploy-vault.dto.ts` around lines 1 - 26, Update the
DeployVaultDto (class DeployVaultDto) to include class-transformer decorators:
add `@Type`(() => Number) to roiPercentage to coerce numeric strings to numbers
and add appropriate string transformation decorators (e.g., trim/normalize via
`@Transform`) to admin, token, usdc, callerPublicKey and to enabled if needed for
boolean coercion so incoming string values are normalized/converted before
validation; then enable transformation globally by updating the ValidationPipe
instantiation (ValidationPipe in main.ts) to include transform: true (e.g., {
whitelist: true, transform: true }) so the class-transformer decorators are
executed during request validation.
| }); | ||
|
|
||
| const tx = await (client)[method](args); | ||
| const tx = await client[method](args); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In @stellar/stellar-sdk 14.6.1, what is the documented return shape of methods invoked on a client created with contract.Client.from({...}), and is calling them through client[method](args) with an arbitrary runtime string a supported pattern?
💡 Result:
-
Documented return shape (v14.6.1):
contract.Client.from({...})builds aClientwhose contract functions are added as methods; each contract method returns anAssembledTransaction(in practice youawaitthe call, since assembly/simulation is async). TheAssembledTransactionis the “workhorse” object you use to inspect the call (resultgetter), serialize (toJSON()), and (for write calls) sign/send (signAndSend()→SentTransaction, which also has aresultgetter). [1][2] -
Is
client[method](args)with an arbitrary runtime string supported? The docs describe calling named methods (e.g.client.myReadMethod(...)) that are generated from the contract spec. The implementation attaches methods dynamically under a sanitized identifier of the contract function name, not a general “invoke-by-string” API; so while bracket-access can work in JavaScript when the key matches the generated property name, calling via an arbitrary runtime string is not a documented/supported pattern (and is not type-safe, and can fail if the name isn’t exactly the generated/sanitized one). [1]
Sources: [1] [2]
🏁 Script executed:
cat -n apps/core/src/soroban/soroban.service.ts | head -80Repository: Trustless-Work/tokenization-private-credit
Length of output: 2116
Validate the contract method name before invoking it.
client[method](args) with method: string bypasses type safety and ignores the @stellar/stellar-sdk documented pattern (calling named methods generated from the contract spec). A typo or unsupported method name will throw an unhandled runtime error instead of a proper HTTP error. Per coding guidelines, use a NestJS exception for invalid input.
Add validation to resolve and verify the method exists before invocation:
Suggested approach
-import { Injectable } from '@nestjs/common';
+import { BadRequestException, Injectable } from '@nestjs/common';
import { contract, Networks } from '@stellar/stellar-sdk';
`@Injectable`()
export class SorobanService {
private readonly rpcUrl: string;
private readonly networkPassphrase: string;
constructor() {
this.rpcUrl = process.env.SOROBAN_RPC_URL!;
this.networkPassphrase = Networks.TESTNET;
}
+
+ private getContractMethod<TResult>(
+ client: object,
+ method: string,
+ ): (args: Record<string, unknown>) => Promise<TResult> {
+ const candidate: unknown = Reflect.get(client, method);
+ if (typeof candidate !== 'function') {
+ throw new BadRequestException(`Unsupported contract method: ${method}`);
+ }
+
+ return candidate.bind(client) as (
+ args: Record<string, unknown>,
+ ) => Promise<TResult>;
+ }
async buildDeployTransaction(
wasmHash: string,
args: Record<string, unknown>,
callerPublicKey: string,
@@
async buildContractCallTransaction(
contractId: string,
method: string,
args: Record<string, unknown>,
callerPublicKey: string,
): Promise<string> {
@@
- const tx = await client[method](args);
+ const invoke = this.getContractMethod<{ toXDR(): string }>(client, method);
+ const tx = await invoke(args);
return tx.toXDR();
}
async readContractState(
@@
): Promise<unknown> {
@@
- const result = await client[method](args);
+ const invoke = this.getContractMethod<{ result: unknown }>(client, method);
+ const result = await invoke(args);
return result.result;
}
}Applies to lines 43 and 61.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/core/src/soroban/soroban.service.ts` at line 43, Validate that the
requested contract method exists and is callable before invoking dynamic calls
like client[method](args) (applies to both occurrences where
client[method](args) is used); resolve the property from client, check typeof
resolved === 'function', and if not throw a NestJS BadRequestException (or
HttpException) with a clear message including the supplied method name,
otherwise call the function safely (e.g., const fn = (client as any)[method];
return await fn(args)). Ensure you reference the variables client and method and
replace unguarded dynamic invocation sites with this guarded pattern so typos or
unsupported method names produce a proper HTTP error instead of an unhandled
runtime exception.
dedabe8 to
29d9658
Compare
- Add deploy endpoints for token-factory and vault contracts - Add vault module with availability-for-exchange write endpoint - Add readContractState method to SorobanService for view functions - Add CI pipeline for lint, format, and build checks - Update core README with required setup commands
- Add ClaimDto with contractId, beneficiary, and callerPublicKey validation
- Add POST /vault/claim endpoint returning { unsignedXdr }
- Implement VaultService.claim method using buildContractCallTransaction
- POST /vault/claim for token-to-USDC exchange - GET endpoints for overview, preview-claim, is-enabled, usdc-balance, total-redeemed, admin, roi-percentage, token-address, usdc-address
cabcac6 to
bd2111f
Compare
Closes #17
Summary by CodeRabbit
New Features
Chores
Modules