refactoring (please read the description) + initial POST support #3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
tcsenpai
commented
Mar 21, 2024
tcsenpai
commented
- Refactored most of serverHandlers.ts to be modular
- POST support (theoretical) on backend + DAHR start
tcsenpai
pushed a commit
that referenced
this pull request
Oct 10, 2025
## Documentation - Add comprehensive GitBook-style Storage Programs documentation in docs/storage_features/ - overview.md: Introduction and core concepts - getting-started.md: Quick start guide with examples - operations.md: Complete CRUD operations reference - access-control.md: Permission system deep dive - rpc-queries.md: RPC query optimization patterns - examples.md: 8 real-world implementation examples - api-reference.md: Complete API documentation ## Bug Fixes ### CRITICAL #1: Circular reference stack overflow (validateStorageProgramSize.ts:54-76) - Added WeakSet-based circular reference detection in validateNestingDepth() - Prevents infinite recursion when users submit objects with circular references - Impact: Prevented DoS attack vector via stack overflow ### CRITICAL #2: Size limit bypass via merge (handleGCR.ts:396-414) - Added merged size validation BEFORE database save in WRITE operation - Users could previously bypass 128KB limit with multiple WRITE calls - Now validates merged data size and rejects if exceeding limit - Impact: Prevented storage abuse and enforced storage limits correctly ### CRITICAL #3: Variable shadowing in RPC (manageNodeCall.ts:136-138, 223-229) - Fixed variable shadowing in getStorageProgram endpoint (data → responseData) - Fixed variable shadowing in getTweet endpoint (data → tweetData) - Outer 'data' variable was being shadowed causing incorrect response values - Impact: Fixed incorrect RPC responses ### MAJOR #4: Database field name mismatch (handleGCR.ts:323, 338, manageNodeCall.ts:199) - Fixed GCRMain entity queries using incorrect field name ('address' → 'pubkey') - Updated CREATE operation to use correct entity fields with proper initialization - Updated RPC endpoint query to use 'pubkey' field - Impact: Fixed database query failures preventing all Storage Programs operations 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
tcsenpai
pushed a commit
that referenced
this pull request
Nov 8, 2025
This commit implements all autofixable issues plus race condition mitigation: CRITICAL FIXES: - Issue #1: Made handleMessage async to support await operations (signalingServer.ts:156) - Issue #3: Removed double increment of offline message count (signalingServer.ts:412) - Issue #2: Added mutex locking to prevent race conditions on shared state Maps * Installed async-mutex package * Protected senderNonces with nonceMutex for transaction uniqueness * Protected offlineMessageCounts with countMutex for rate limiting * Atomic check-and-increment/decrement operations HIGH PRIORITY FIXES: - Issue #5: Reversed blockchain/DB storage order (DB first for easier rollback) - Issue #6: Added L2PS decryption error handling with try-catch and null checks (handleL2PS.ts:56-72) MEDIUM PRIORITY FIXES: - Issue #7: Added L2PS mempool error handling (handleL2PS.ts:101-111) LOW PRIORITY FIXES: - Issue #8: Added pagination support to L2PSHashes.getAll() (l2ps_hashes.ts:152-169) - Issue #9: Added non-null assertions for type safety (l2ps_hashes.ts:97, 125, 161) - Issue #10: Changed "delivered" to "sent" for semantic accuracy * Updated status in signalingServer.ts * Updated OfflineMessage entity to include "sent" status * No migration needed (synchronize: true handles schema update) All changes include REVIEW comments for code review tracking. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
tcsenpai
pushed a commit
that referenced
this pull request
Nov 10, 2025
CRITICAL FIX: rollbackMerkleTreeToBlock now uses transaction wrapper to ensure atomicity during chain reorgs. PROBLEM: - Two separate DB operations (update commitments, delete tree states) - If second operation failed, first would already be committed - Result: inconsistent Merkle tree state during rollbacks SOLUTION: - Wrapped both operations in dataSource.transaction() - All operations succeed or all rollback together - Prevents partial rollback corruption Impact: Fixes Issue #3 from PR_TOFIX.md File: src/features/zk/merkle/updateMerkleTreeAfterBlock.ts:115-174
tcsenpai
pushed a commit
that referenced
this pull request
Nov 10, 2025
Issue #3 resolved with transaction wrapper in rollbackMerkleTreeToBlock.
tcsenpai
pushed a commit
that referenced
this pull request
Nov 19, 2025
Addresses 5 CRITICAL issues from CodeRabbit review: 1. **CRITICAL #1-2: Nullifier Race Condition** - File: GCRIdentityRoutines.ts - Issue: Race between proof verification and nullifier marking - Solution: Wrap markNullifierUsed() in try-catch, rely on database primary key constraint for atomicity - Gracefully handle concurrent double-attestation attempts 2. **CRITICAL #3: Shared State Updates Inside Transaction** - File: chain.ts - Issue: Memory state updated before transaction commits - Solution: Defer getSharedState updates until AFTER transaction - Prevents memory corruption if transaction rolls back 3. **CRITICAL #4: Transaction Insertions Bypass Manager** - File: chain.ts - Issue: insertTransaction() uses direct repository, bypassing transaction - Solution: Use transactionalEntityManager.save() directly in loop - Ensures all saves are part of same transaction 4. **CRITICAL #5: Missing Transaction Wrapper** - File: updateMerkleTreeAfterBlock.ts - Issue: Multiple DB operations without transaction wrapper - Solution: Accept optional EntityManager parameter - Wrap in own transaction if standalone, use provided manager if available - Ensures atomicity: all tree operations succeed or fail together Additional fixes: - Fixed duplicate GLOBAL_TREE_ID definition bug - Fixed stats property access (totalLeaves -> leafCount, currentRoot -> root) All database operations now properly wrapped in transactions for consistency and rollback safety. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
tcsenpai
pushed a commit
that referenced
this pull request
Nov 19, 2025
Fixed all autofixable issues from second CodeRabbit review after
validating that all 11 previous fixes were correct.
CRITICAL Fixes (2):
- Fix timestamp overflow: Changed UsedNullifier.timestamp to bigint
with TypeORM transformer to prevent overflow with Date.now() milliseconds
- Fix broken singleton: MerkleTreeManager only assigned to global after
successful initialization to prevent corrupted state on error
HIGH Priority Fixes (2):
- Remove extra poseidon2 argument from verifyProof() - API only accepts
one argument (MerkleProof object)
- Throw error instead of returning null in getProofForCommitment() to
distinguish "not found" from "system error"
MEDIUM Priority Fixes (3):
- Fix path resolution: Use import.meta.url in test_snarkjs_bun.ts for
reliable path resolution independent of working directory
- Fix path traversal false positives: Check ".." as path segment with
normalize().startsWith('..') instead of substring match
- Add CI exit codes: test_snarkjs_bun.ts now calls process.exit() with
proper codes for CI/CD integration
LOW Priority Fixes (2):
- Fix invalid hex string example: Changed "0x5e6f7g8h..." to valid hex
and clarified length is 64 hex digits + "0x" prefix
- Fix overstated performance claim: Changed "~5x faster" to "30-50%
faster" with accurate proof size comparison
Note: Skipped CRITICAL #3 (Bun-specific APIs) as this project uses Bun
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
tcsenpai
pushed a commit
that referenced
this pull request
Nov 19, 2025
**HIGH #1: Transaction Boundary with MerkleTreeManager** - Updated MerkleTreeManager.saveToDatabase() to accept optional EntityManager - Pass transactional manager through call chain in updateMerkleTreeAfterBlock - Ensures all Merkle tree DB operations occur within transaction boundary **HIGH #2 & #3: TypeORM QueryBuilder Using Column Names** - Fixed rollbackMerkleTreeToBlock() update query to use property names - Added alias 'commitment' and used blockNumber/treeId instead of column names - Fixed delete query to use property names (blockNumber, treeId) **HIGH #4: Weak Commitment Hash Validation** - Added regex validation for 64-char hex pattern (with optional 0x prefix) - Added validation for numeric string pattern - Prevents malformed inputs from passing simulate mode validation **MEDIUM #1: Variable Shadowing and Redundant DataSource Call** - Removed redundant Datasource.getInstance() call on line 712 - Reused existing dataSource variable from line 707 - Eliminates variable shadowing and improves code clarity **MEDIUM #2: Hex String Documentation Ambiguity** - Clarified JSDoc for IdentityCommitment.commitmentHash field - Explicitly states "64 hex digits with optional 0x prefix" - Specifies total length: 66 chars with prefix, 64 without Files modified: - src/features/zk/merkle/MerkleTreeManager.ts - src/features/zk/merkle/updateMerkleTreeAfterBlock.ts - src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts - src/model/entities/GCRv2/IdentityCommitment.ts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
tcsenpai
pushed a commit
that referenced
this pull request
Nov 19, 2025
Implemented 6 of 7 authorized fixes from PR_REVIEW_ROUND5.md: ✅ HIGH #1: Removed incorrect treeId filter from rollbackMerkleTreeToBlock - Fixed bug introduced in Round 4 - IdentityCommitment entity has no treeId field - All commitments belong to global tree ✅ HIGH #4: Standardized timestamp handling to string format - Changed Date.now() to Date.now().toString() at line 745 - Matches IdentityCommitment.timestamp type (bigint/string) ✅ MEDIUM #2: Added provider/timestamp validation - Validates provider field (string, non-empty) - Validates timestamp field (number type) ✅ MEDIUM #3: Added ZK attestation format validation - Type checks for nullifier_hash, merkle_root, proof, public_signals - Format validation for nullifier_hash (hex pattern) ✅ HIGH #3: Implemented initialization retry backoff - 5-second backoff after initialization failures - Prevents retry storms from crashing system - Clear error messages with remaining backoff time ✅ HIGH #2: Refactored /zk/merkle-root endpoint - Now uses singleton MerkleTreeManager for consistency - Fast in-memory access for root and leafCount - Consistent with /zk/merkle/proof endpoint CRITICAL #1 (optimistic locking replacement) deferred for next commit due to complexity and multi-file refactoring requirements. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
tcsenpai
pushed a commit
that referenced
this pull request
Nov 19, 2025
…le backoff recovery Issue: HIGH #3 fix from Round 5 introduced a bug where initializationPromise remained set after failures, preventing the backoff retry logic from ever executing. This created a permanent failure state. Root cause: In catch block, kept promise set to "prevent concurrent retries", but this prevented subsequent calls from reaching the backoff logic at lines 72-86. Fix: Clear initializationPromise = null in catch block so next call can: 1. Check lastInitializationError timestamp 2. Apply exponential backoff if within backoff window 3. Attempt new initialization if backoff expired Files changed: - src/libs/network/server_rpc.ts:104-118 - Added initializationPromise = null in catch block (line 110) - Removed misleading comments about preventing concurrent retries - Now allows proper backoff recovery after initialization failures Testing: lint:fix shows no new errors in modified file Resolves: CodeRabbit Round 6 Issue #2 (CRITICAL) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
tcsenpai
pushed a commit
that referenced
this pull request
Nov 19, 2025
…uit path Issue #1: Type naming inconsistency (publicSignals vs public_signals) - Standardized ProofGenerationResult to use snake_case (public_signals) - Extracted shared Groth16Proof interface to eliminate duplication - Updated IdentityAttestationPayload to reference Groth16Proof interface - Ensures consistent naming across ZK type system Issue #3: Missing IC array validation in BunSnarkjsWrapper - Added Array.isArray() check for vk_verifier.IC - Added length > 0 validation before accessing IC[0] at line 79 - Updated error message to indicate invalid/missing IC array - Prevents runtime errors from invalid verification key structure Issue #6 (part 1): Incorrect circomlib include path - Fixed: circomlib/circuits/poseidon.circom → circomlib/poseidon.circom - Correct path for Circom 2.x iden3/circomlib compatibility - Enables successful circuit compilation Files changed: - src/features/zk/types/index.ts - Extracted Groth16Proof interface (lines 18-28) - Updated IdentityAttestationPayload to use Groth16Proof (line 40) - Changed ProofGenerationResult.publicSignals → public_signals (line 117) - src/features/zk/proof/BunSnarkjsWrapper.ts:54-56 - Added Array.isArray(vk_verifier.IC) && vk_verifier.IC.length === 0 - Updated error message for IC validation - src/features/zk/circuits/identity.circom:3 - Fixed include path to circomlib/poseidon.circom Testing: lint:fix shows no new errors in modified files Resolves: CodeRabbit Round 6 Issues #1, #3, #6 (part 1) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.