Merge pull request #76 from Michaelkingsdev/normalize-portfolio

fix: enforce strict allocation types across risk management pipeline

Author: ritik4ever

backend/src/monitoring/rebalancer.ts

@@ -69,7 +69,15 @@ export class RebalancingService {
 
             if (needsRebalance) {
                 logger.info(`Portfolio ${portfolioId} needs rebalancing – enqueueing job`)
-                const riskCheck = riskManagementService.shouldAllowRebalance(portfolio, prices)
+
+                // Use the stored portfolio (Record<string, number> allocations) for risk checks,
+                // NOT the UI response from stellarService.getPortfolio() which has an array shape.
+                const storedPortfolio = await portfolioStorage.getPortfolio(portfolioId)
+                if (!storedPortfolio) {
+                    logger.warn(`Portfolio ${portfolioId} not found in storage during risk check`)
+                    return
+                }
+                const riskCheck = riskManagementService.shouldAllowRebalance(storedPortfolio, prices)
 
                 if (riskCheck.allowed) {
                     // Enqueue a rebalance job rather than executing inline
@@ -99,7 +107,7 @@ export class RebalancingService {
                         gasUsed: '0 XLM',
                         status: 'failed',
                         prices,
-                        portfolio,
+                        portfolio: storedPortfolio,
                     })
                 }
             }

backend/src/queue/workers/portfolioCheckWorker.ts

@@ -69,16 +69,20 @@ export async function processPortfolioCheckJob(
                 continue
             }
 
-            // Check cooldown
-            const p = await stellarService.getPortfolio(portfolio.id)
-            const cooldownCheck = CircuitBreakers.checkCooldownPeriod(p.lastRebalance)
+            // Use the stored portfolio directly for risk checks.
+            // stellarService.getPortfolio() returns a UI response with allocations as an
+            // array, which would corrupt weight calculations in shouldAllowRebalance.
+            const storedPortfolio = portfolio  // already the stored shape from getAllPortfolios()
+
+            // Check cooldown using last-rebalance timestamp from stored portfolio
+            const cooldownCheck = CircuitBreakers.checkCooldownPeriod(storedPortfolio.lastRebalance)
             if (!cooldownCheck.safe) {
                 skipped++
                 continue
             }
 
-            // Risk management
-            const riskCheck = riskManagementService.shouldAllowRebalance(p, prices)
+            // Risk management — pass stored portfolio with Record<string, number> allocations
+            const riskCheck = riskManagementService.shouldAllowRebalance(storedPortfolio, prices)
             if (!riskCheck.allowed) {
                 logger.warn('[WORKER:portfolio-check] Rebalance blocked by risk management', {
                     portfolioId: portfolio.id,
@@ -89,7 +93,7 @@ export async function processPortfolioCheckJob(
             }
 
             // Concentration risk
-            const concentrationCheck = CircuitBreakers.checkConcentrationRisk(p.allocations)
+            const concentrationCheck = CircuitBreakers.checkConcentrationRisk(storedPortfolio.allocations)
             if (!concentrationCheck.safe) {
                 skipped++
                 continue

backend/src/services/circuitBreakers.ts

@@ -82,22 +82,22 @@ export class CircuitBreakers {
         return { safe: true }
     }
 
-    static checkConcentrationRisk(allocations: any[]): { safe: boolean, reason?: string } {
+    static checkConcentrationRisk(allocations: Record<string, number>): { safe: boolean, reason?: string } {
         const maxSingleAsset = 80 // 80% maximum for any single asset
         const minDiversification = 1 // Minimum number of assets
 
-        // Check maximum concentration
-        for (const allocation of allocations) {
-            if (allocation.current > maxSingleAsset) {
+        // Check maximum concentration (values are target percentages, 0-100)
+        for (const [asset, percentage] of Object.entries(allocations)) {
+            if (percentage > maxSingleAsset) {
                 return {
                     safe: false,
-                    reason: `Concentration risk: ${allocation.asset} represents ${allocation.current.toFixed(1)}% of portfolio`
+                    reason: `Concentration risk: ${asset} represents ${percentage.toFixed(1)}% of portfolio`
                 }
             }
         }
 
         // Check minimum diversification
-        const activeAssets = allocations.filter(a => a.current > 1).length
+        const activeAssets = Object.values(allocations).filter(pct => pct > 1).length
         if (activeAssets < minDiversification) {
             return {
                 safe: false,

backend/src/services/databaseService.ts

@@ -6,6 +6,7 @@ import type { RebalanceEvent } from './rebalanceHistory.js'
 import { getFeatureFlags } from '../config/featureFlags.js'
 import { ConflictError } from '../types/index.js'
 import { logger } from '../utils/logger.js'
+import type { Portfolio } from '../types/index.js'
 
 // ─────────────────────────────────────────────
 // Exported type used by rebalanceHistory.ts
@@ -24,20 +25,6 @@ export interface RebalanceHistoryQueryOptions {
 // ─────────────────────────────────────────────
 // Types (mirrored from portfolioStorage.ts)
 // ─────────────────────────────────────────────
-
-export interface Portfolio {
-    id: string
-    userAddress: string
-    allocations: Record<string, number>
-    threshold: number
-    slippageTolerancePercent?: number
-    balances: Record<string, number>
-    totalValue: number
-    createdAt: string
-    lastRebalance: string
-    version: number
-}
-
 interface PortfolioRow {
     id: string
     user_address: string
@@ -247,7 +234,7 @@ function rowToPortfolio(row: PortfolioRow): Portfolio {
         userAddress: row.user_address,
         allocations: safeJsonParse(row.allocations, {}, `portfolio(${row.id}).allocations`),
         threshold: row.threshold,
-        slippageTolerancePercent: row.slippage_tolerance_percent ?? 1,
+        slippageTolerance: row.slippage_tolerance_percent ?? 1,
         balances: safeJsonParse(row.balances, {}, `portfolio(${row.id}).balances`),
         totalValue: row.total_value,
         createdAt: row.created_at,

backend/src/services/portfolioStorage.ts

@@ -1,6 +1,7 @@
 import { isDbConfigured } from '../db/client.js'
 import * as portfolioDb from '../db/portfolioDb.js'
 import { randomUUID } from 'node:crypto'
+import type { Portfolio } from '../types/index.js'
 
 const SLIPPAGE_MIN = 0.5
 const SLIPPAGE_MAX = 5
@@ -9,17 +10,7 @@ function clampSlippageTolerance(p: number): number {
     return Math.max(SLIPPAGE_MIN, Math.min(SLIPPAGE_MAX, p))
 }
 
-interface Portfolio {
-    id: string
-    userAddress: string
-    allocations: Record<string, number>
-    threshold: number
-    slippageTolerance?: number
-    balances: Record<string, number>
-    totalValue: number
-    createdAt: string
-    lastRebalance: string
-}
+
 
 const useCache = process.env.USE_MEMORY_CACHE === 'true'
 
@@ -56,7 +47,8 @@ class PortfolioStorage {
             balances: {},
             totalValue: 0,
             createdAt: new Date().toISOString(),
-            lastRebalance: new Date().toISOString()
+            lastRebalance: new Date().toISOString(),
+            version: 1
         }
         if (isDbConfigured()) {
             await portfolioDb.dbCreatePortfolio(id, userAddress, allocations, threshold, {}, 0)
@@ -83,7 +75,8 @@ class PortfolioStorage {
             balances: currentBalances,
             totalValue,
             createdAt: new Date().toISOString(),
-            lastRebalance: new Date().toISOString()
+            lastRebalance: new Date().toISOString(),
+            version: 1
         }
         if (isDbConfigured()) {
             await portfolioDb.dbCreatePortfolio(
@@ -159,10 +152,5 @@ class PortfolioStorage {
         this.portfolios.clear()
     }
 }
-/**
- * portfolioStorage.ts
- *
- * Backward-compatible re-export: all callers that import `portfolioStorage`
- * now transparently use the SQLite-backed DatabaseService singleton.
- */
-export { databaseService as portfolioStorage, type Portfolio } from './databaseService.js'
+export { databaseService as portfolioStorage } from './databaseService.js'
+export type { Portfolio } from '../types/index.js'

backend/src/services/rebalanceHistory.ts

@@ -61,7 +61,8 @@ export class RebalanceHistoryService {
         toAsset?: string
         amount?: number
         prices?: PricesMap
-        portfolio?: any
+        /** Stored portfolio record. Must have `allocations` as a `Record<string, number>`. */
+        portfolio?: { allocations: Record<string, number> }
         eventSource?: RebalanceEvent['eventSource']
         onChainConfirmed?: boolean
         onChainEventType?: string

backend/src/services/rebalanceLock.ts

@@ -67,10 +67,10 @@ export class RebalanceLockService {
 
         if (this.useRedis && this.redis) {
             try {
-                // SET resource_name my_random_value NX PX ttl
+                // SET resource_name my_random_value PX ttl
                 // NX = set if not exist
                 // PX = expire time in milliseconds
-                const result = await this.redis.set(lockKey, 'locked', 'PX', ttlMs, 'NX')
+
                 return result === 'OK'
             } catch (error) {
                 logger.error(`[LOCK_SERVICE] Failed to acquire Redis lock for ${portfolioId}`, {

backend/src/services/riskManagements.ts

@@ -107,7 +107,7 @@ export class RiskManagementService {
     }
 
     analyzePortfolioRisk(
-        allocationsInput: Record<string, number> | Array<{ asset: string, target?: number, current?: number, percentage?: number }>,
+        allocationsInput: Record<string, number>,
         _prices: PricesMap
     ): RiskMetrics {
         const weights = this.normalizeAllocations(allocationsInput)
@@ -157,7 +157,7 @@ export class RiskManagementService {
         }
     }
 
-    shouldAllowRebalance(portfolio: any, prices: PricesMap): {
+    shouldAllowRebalance(portfolio: { allocations: Record<string, number> }, prices: PricesMap): {
         allowed: boolean
         reason?: string
         reasonCode?: RiskDecisionReasonCode
@@ -385,18 +385,9 @@ export class RiskManagementService {
     }
 
     private normalizeAllocations(
-        allocationsInput: Record<string, number> | Array<{ asset: string, target?: number, current?: number, percentage?: number }>
+        allocationsInput: Record<string, number>
     ): Record<string, number> {
-        let raw: Record<string, number> = {}
-
-        if (Array.isArray(allocationsInput)) {
-            allocationsInput.forEach(item => {
-                const value = item.current ?? item.target ?? item.percentage ?? 0
-                raw[item.asset] = value
-            })
-        } else {
-            raw = { ...allocationsInput }
-        }
+        let raw: Record<string, number> = { ...allocationsInput }
 
         const cleaned = Object.entries(raw).reduce<Record<string, number>>((acc, [asset, value]) => {
             const numeric = Number(value)

backend/src/types/index.ts

@@ -22,6 +22,24 @@ export interface HistoricalPrice {
     version: number
 }
 
+
+/**
+ * Per-asset allocation item returned by the API / UI layer.
+ * This is the array-element shape produced by StellarService.getPortfolio()
+ * and MUST be converted via toStoredAllocations() before being passed to any
+ * risk-management method.
+ */
+export interface UIAllocation {
+    asset: string
+    /** Target weight as a percentage (0-100) */
+    target: number
+    /** Current weight as a percentage (0-100) */
+    current: number
+    amount?: number
+    balance?: number
+    price?: number
+}
+
 // Thrown when an update targets a stale portfolio version
 export class ConflictError extends Error {
     readonly currentVersion: number

backend/src/utils/allocationUtils.ts

@@ -0,0 +1,35 @@
+import type { UIAllocation } from '../types/index.js'
+
+/**
+ * allocation format (`Record<string, number>`).
+ */
+export function isStoredAllocations(v: unknown): v is Record<string, number> {
+    if (v === null || typeof v !== 'object' || Array.isArray(v)) return false
+    return Object.values(v as object).every(x => typeof x === 'number')
+}
+
+/**
+ * @example
+ * // From stored portfolio
+ * toStoredAllocations({ XLM: 40, BTC: 30, ETH: 20, USDC: 10 })
+ * // → { XLM: 40, BTC: 30, ETH: 20, USDC: 10 }
+ *
+ * @example
+ * // From UI response
+ * toStoredAllocations([{ asset: 'XLM', target: 40, current: 38 }])
+ * // → { XLM: 40 }
+ */
+export function toStoredAllocations(
+    input: Record<string, number> | UIAllocation[]
+): Record<string, number> {
+    if (Array.isArray(input)) {
+        const result: Record<string, number> = {}
+        for (const item of input) {
+            if (typeof item.asset === 'string') {
+                result[item.asset] = item.target
+            }
+        }
+        return result
+    }
+    return { ...input }
+}