From 1223e9b91eba635429ae941d68d6af531caf23ea Mon Sep 17 00:00:00 2001 From: adeniran19-maker <288629594+adeniran19-maker@users.noreply.github.com> Date: Sat, 29 Aug 2026 14:45:32 +0100 Subject: [PATCH] feat(soroban): detect storage access inside loops (#875) --- .../storage/storage-in-loop-analyzer.ts | 196 ++++++++++++++++++ packages/rules/soroban/src/index.ts | 1 + packages/rules/soroban/src/storage/index.ts | 1 + .../src/storage/storage-in-loop-rule.ts | 39 ++++ .../soroban/tests/storage-in-loop.spec.ts | 68 ++++++ 5 files changed, 305 insertions(+) create mode 100644 packages/analyzers/soroban/storage/storage-in-loop-analyzer.ts create mode 100644 packages/rules/soroban/src/storage/index.ts create mode 100644 packages/rules/soroban/src/storage/storage-in-loop-rule.ts create mode 100644 packages/rules/soroban/tests/storage-in-loop.spec.ts diff --git a/packages/analyzers/soroban/storage/storage-in-loop-analyzer.ts b/packages/analyzers/soroban/storage/storage-in-loop-analyzer.ts new file mode 100644 index 00000000..51432d35 --- /dev/null +++ b/packages/analyzers/soroban/storage/storage-in-loop-analyzer.ts @@ -0,0 +1,196 @@ +import { + maskNonCode, + createLineResolver, + extractFunctions, + extractArgs, + splitArgs, + blockStackAt, + isInLoop, +} from '../common/source-utils'; +export type LoopBoundType = 'bounded_range' | 'collection_iterator' | 'unbounded' | 'dynamic_condition'; + +export interface LoopContext { + loopType: 'for' | 'while' | 'loop'; + boundType: LoopBoundType; + boundExpression: string; + line: number; +} + +export function classifyLoopHeader(precedingSource: string): LoopContext { + const forMatch = precedingSource.match(/for\s+([A-Za-z0-9_(),\s]+)\s+in\s+([^{]+)/); + if (forMatch) { + const expr = forMatch[2].trim(); + const isRange = /\d+\s*\.\.\s*=?\s*\d+/.test(expr); + return { + loopType: 'for', + boundType: isRange ? 'bounded_range' : 'collection_iterator', + boundExpression: expr, + line: 0, + }; + } + + const whileMatch = precedingSource.match(/while\s+([^{]+)/); + if (whileMatch) { + const expr = whileMatch[1].trim(); + const isTrue = expr === 'true'; + return { + loopType: 'while', + boundType: isTrue ? 'unbounded' : 'dynamic_condition', + boundExpression: expr, + line: 0, + }; + } + + return { + loopType: 'loop', + boundType: 'unbounded', + boundExpression: 'unbounded loop', + line: 0, + }; +} + +export type StorageOpType = 'read' | 'write'; +export type StorageScope = 'instance' | 'persistent' | 'temporary' | 'unknown'; + +export interface StorageInLoopSite { + fn: string; + opType: StorageOpType; + scope: StorageScope; + method: string; + key: string; + line: number; + offset: number; + loopContext: LoopContext; + severity: 'critical' | 'high' | 'medium'; + estimatedResourceImpact: { + cpuInstructions: number; + storageBytes: number; + }; + message: string; + suggestion: string; +} + +export interface StorageInLoopReport { + sites: StorageInLoopSite[]; + totalReadsInLoops: number; + totalWritesInLoops: number; + estimatedTotalCpuMultiplier: number; + recommendations: string[]; +} + +const STORAGE_WRITE_METHODS = new Set(['set', 'put', 'extend_ttl']); +const STORAGE_READ_METHODS = new Set(['get', 'has', 'get_unchecked']); + +// Regex targeting Soroban storage calls: e.g. env.storage().instance().set(...) or storage().persistent().get(...) +const STORAGE_CALL_REGEX = /\bstorage\s*\(\s*\)\s*\.\s*(instance|persistent|temporary)\s*\(\s*\)\s*\.\s*([a-zA-Z0-9_]+)\s*\(/g; + +/** + * Detect storage read and write operations inside loops and compute resource estimates. + */ +export function detectStorageInLoops(source: string): StorageInLoopSite[] { + const masked = maskNonCode(source); + const lineOf = createLineResolver(source); + const functions = extractFunctions(masked, source); + const sites: StorageInLoopSite[] = []; + + for (const fn of functions) { + const body = masked.slice(fn.bodyStart, fn.bodyEnd); + let m: RegExpExecArray | null; + + while ((m = STORAGE_CALL_REGEX.exec(body)) !== null) { + const offset = fn.bodyStart + m.index; + const stack = blockStackAt(masked, fn.bodyStart, offset); + + if (!isInLoop(stack)) { + continue; + } + + const scope = (m[1] as StorageScope) || 'unknown'; + const method = m[2]; + const isWrite = STORAGE_WRITE_METHODS.has(method); + const isRead = STORAGE_READ_METHODS.has(method); + + if (!isWrite && !isRead) { + continue; + } + + const opType: StorageOpType = isWrite ? 'write' : 'read'; + + // Find enclosing loop frame and classify bounds + const loopFrame = stack.slice().reverse().find((f) => f.kind === 'loop'); + const loopHeaderSnippet = loopFrame + ? source.slice(Math.max(fn.bodyStart, loopFrame.start - 80), loopFrame.start) + : ''; + + const loopContext = classifyLoopHeader(loopHeaderSnippet); + loopContext.line = loopFrame ? lineOf(loopFrame.start) : lineOf(offset); + + // Extract storage key argument + const openParen = offset + m[0].length - 1; + const argsText = extractArgs(masked, source, openParen).text; + const args = splitArgs(argsText); + const key = args.length > 0 ? args[0] : 'unknown_key'; + + // Severity and resource estimation + const isUnbounded = loopContext.boundType === 'unbounded' || loopContext.boundType === 'dynamic_condition'; + const severity = isWrite + ? (isUnbounded ? 'critical' : 'high') + : (isUnbounded ? 'high' : 'medium'); + + const multiplier = loopContext.boundType === 'bounded_range' ? 5 : 20; + const baseCpu = isWrite ? 25_000 : 10_000; + const baseBytes = isWrite ? 1_000 : 500; + + sites.push({ + fn: fn.name, + opType, + scope, + method, + key, + line: lineOf(offset), + offset, + loopContext, + severity, + estimatedResourceImpact: { + cpuInstructions: baseCpu * multiplier, + storageBytes: baseBytes * multiplier, + }, + message: `Storage ${opType} (env.storage().${scope}().${method}('${key}')) detected inside a '${loopContext.loopType}' loop (${loopContext.boundType}) in '${fn.name}'.`, + suggestion: isWrite + ? `Buffer state modifications in a local Map or Vec in memory, and perform a single batched storage write after the loop.` + : `Hoist the storage query outside the loop into a local variable if key '${key}' does not change per iteration.`, + }); + } + } + + return sites.sort((a, b) => a.line - b.line); +} + +/** + * Full analysis entry point. + */ +export function analyzeStorageInLoops(source: string): StorageInLoopReport { + const sites = detectStorageInLoops(source); + const reads = sites.filter((s) => s.opType === 'read'); + const writes = sites.filter((s) => s.opType === 'write'); + + const recommendations: string[] = []; + if (writes.length > 0) { + recommendations.push( + `Detected ${writes.length} storage write(s) inside loops. Batch storage updates to avoid expensive ledger write serialization and rent growth.`, + ); + } + if (reads.length > 0) { + recommendations.push( + `Detected ${reads.length} storage read(s) inside loops. Cache read values in local memory before entering loop bodies.`, + ); + } + + return { + sites, + totalReadsInLoops: reads.length, + totalWritesInLoops: writes.length, + estimatedTotalCpuMultiplier: sites.reduce((sum, s) => sum + s.estimatedResourceImpact.cpuInstructions, 0), + recommendations, + }; +} diff --git a/packages/rules/soroban/src/index.ts b/packages/rules/soroban/src/index.ts index d069a936..2500666d 100644 --- a/packages/rules/soroban/src/index.ts +++ b/packages/rules/soroban/src/index.ts @@ -13,3 +13,4 @@ export * from './prioritization'; export * from './functions'; export * from './resources'; export * from './tokens'; +export * from './storage'; diff --git a/packages/rules/soroban/src/storage/index.ts b/packages/rules/soroban/src/storage/index.ts new file mode 100644 index 00000000..f08e618b --- /dev/null +++ b/packages/rules/soroban/src/storage/index.ts @@ -0,0 +1 @@ +export * from './storage-in-loop-rule'; diff --git a/packages/rules/soroban/src/storage/storage-in-loop-rule.ts b/packages/rules/soroban/src/storage/storage-in-loop-rule.ts new file mode 100644 index 00000000..92075a95 --- /dev/null +++ b/packages/rules/soroban/src/storage/storage-in-loop-rule.ts @@ -0,0 +1,39 @@ +/** + * Rule: soroban-storage-in-loop (#875) + * Detects storage read/write operations executed repeatedly inside loops. + */ +import { + detectStorageInLoops, + StorageInLoopSite, + StorageOpType, + StorageScope, +} from '../../../../analyzers/soroban/storage/storage-in-loop-analyzer'; + +export interface StorageInLoopFinding { + ruleId: 'soroban-storage-in-loop'; + line: number; + message: string; + suggestion: string; + severity: 'critical' | 'high' | 'medium'; + opType: StorageOpType; + scope: StorageScope; + key: string; + boundType: string; + estimatedCpuInstructions: number; +} + +export function detectStorageAccessInsideLoops(source: string): StorageInLoopFinding[] { + const sites = detectStorageInLoops(source); + return sites.map((s: StorageInLoopSite) => ({ + ruleId: 'soroban-storage-in-loop' as const, + line: s.line, + message: s.message, + suggestion: s.suggestion, + severity: s.severity, + opType: s.opType, + scope: s.scope, + key: s.key, + boundType: s.loopContext.boundType, + estimatedCpuInstructions: s.estimatedResourceImpact.cpuInstructions, + })); +} diff --git a/packages/rules/soroban/tests/storage-in-loop.spec.ts b/packages/rules/soroban/tests/storage-in-loop.spec.ts new file mode 100644 index 00000000..71ef2678 --- /dev/null +++ b/packages/rules/soroban/tests/storage-in-loop.spec.ts @@ -0,0 +1,68 @@ +import { detectStorageAccessInsideLoops } from '../src/storage/storage-in-loop-rule'; +import { analyzeStorageInLoops } from '../../../analyzers/soroban/storage/storage-in-loop-analyzer'; + +describe('Detect Storage Access Inside Expensive Soroban Loops (#875)', () => { + const CONTRACT_WITH_STORAGE_LOOPS = ` + pub fn update_user_balances(env: Env, users: Vec
, delta: i128) { + for user in users.iter() { + let mut balance: i128 = env.storage().persistent().get(&user).unwrap_or(0); + balance += delta; + env.storage().persistent().set(&user, &balance); + } + } + + pub fn read_config_in_loop(env: Env, items: Vec) { + for item in items.iter() { + let config: u32 = env.storage().instance().get(&Symbol::new(&env, "cfg")).unwrap_or(0); + } + } + + pub fn clean_single_storage(env: Env, key: Symbol, value: u32) { + env.storage().instance().set(&key, &value); + } + `; + + test('detects both storage reads and writes in loop bodies and distinguishes opTypes', () => { + const findings = detectStorageAccessInsideLoops(CONTRACT_WITH_STORAGE_LOOPS); + + expect(findings.length).toBe(3); // 1 read + 1 write in update_user_balances, 1 read in read_config_in_loop + + const writes = findings.filter((f) => f.opType === 'write'); + const reads = findings.filter((f) => f.opType === 'read'); + + expect(writes.length).toBe(1); + expect(reads.length).toBe(2); + + expect(writes[0].scope).toBe('persistent'); + expect(writes[0].severity).toBe('high'); + expect(writes[0].estimatedCpuInstructions).toBeGreaterThan(0); + }); + + test('extracts loop bound type and storage key', () => { + const findings = detectStorageAccessInsideLoops(CONTRACT_WITH_STORAGE_LOOPS); + + const configRead = findings.find((f) => f.scope === 'instance'); + expect(configRead).toBeDefined(); + expect(configRead?.boundType).toBe('collection_iterator'); + }); + + test('analyzeStorageInLoops aggregates counts and generates recommendations', () => { + const report = analyzeStorageInLoops(CONTRACT_WITH_STORAGE_LOOPS); + + expect(report.totalReadsInLoops).toBe(2); + expect(report.totalWritesInLoops).toBe(1); + expect(report.recommendations.some((r) => r.includes('Batch storage updates'))).toBe(true); + expect(report.recommendations.some((r) => r.includes('Cache read values'))).toBe(true); + }); + + test('returns 0 findings for storage operations outside loops', () => { + const clean = ` + pub fn set_data(env: Env, key: Symbol, val: u32) { + env.storage().instance().set(&key, &val); + } + `; + + const findings = detectStorageAccessInsideLoops(clean); + expect(findings.length).toBe(0); + }); +});