Skip to content

Unified tx#4

Merged
tcsenpai merged 38 commits intomainfrom
unified_tx
Apr 29, 2024
Merged

Unified tx#4
tcsenpai merged 38 commits intomainfrom
unified_tx

Conversation

@tcsenpai
Copy link
Contributor

No description provided.

tcsenpai and others added 30 commits March 28, 2024 18:35
+ comment out unused type sources
+ multichainDispatcher.digest unwrap payload string
@tcsenpai tcsenpai merged commit 82c0d35 into main Apr 29, 2024
@linear linear bot mentioned this pull request Jun 19, 2025
@tcsenpai tcsenpai deleted the unified_tx branch June 24, 2025 14:35
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 Oct 11, 2025
…arity

Address CodeRabbit review concerns #2 and #4:

**Concern #2 - JSON.stringify exception handling (VALID):**
- Add try/catch in getDataSize() to handle serialization errors
- Wrap validateSize() with error handling for graceful failure
- Prevents crashes on circular references or BigInt values
- Returns structured error messages instead of throwing

**Concern #4 - Metadata size calculation clarity (PARTIALLY VALID):**
- Document that transaction handler size is delta (incoming data only)
- Add comments explaining size recalculation in HandleGCR after merge
- Improves code clarity without changing logic (already correct)

Files modified:
- src/libs/blockchain/validators/validateStorageProgramSize.ts
- src/libs/network/routines/transactions/handleStorageProgramTransaction.ts
- src/libs/blockchain/gcr/handleGCR.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 Oct 21, 2025
SECURITY VULNERABILITY FIX: Prevent authentication bypass via case manipulation

Issue: Previous code applied toLowerCase() to ALL addresses including Solana
addresses, which are case-sensitive (base58 encoding). This allowed attackers
to bypass authorization by changing the case of a valid Solana address.

Example Attack:
- Domain authorized for: 8VqZ8cqQ8h9FqF7cXNx5bXKqNz9V8F7h9FqF7cXNx5b
- Attacker uses:       8vqz8cqq8h9fqf7cxnx5bxkqnz9v8f7h9fqf7cxnx5b
- Previous code: ACCEPTED (case-insensitive match)
- Fixed code: REJECTED (case-sensitive for Solana)

Fix:
- Conditional address comparison based on signature type
- Solana addresses: Exact case-sensitive match (base58 standard)
- EVM addresses: Case-insensitive match (EVM standard)

Security Impact: HIGH - Closes critical authentication bypass vulnerability
Affected: All Solana UD domain verifications in production

Reviewer Concern #4
tcsenpai pushed a commit that referenced this pull request Nov 10, 2025
Wrapped block insertion and Merkle tree update in single transaction to prevent silent state divergence. If Merkle update fails, entire block commit rolls back.

- Ensures block and Merkle tree state stay consistent
- Clean failure mode (both succeed or both rollback)
- No silent corruption from partial commits
tcsenpai pushed a commit that referenced this pull request Nov 10, 2025
All critical security issues now resolved:
✅ Issue #1: Circuit privacy (Poseidon(3))
✅ Issue #2: Nullifier TOCTOU race
✅ Issue #3: Merkle rollback race
✅ Issue #4: Block-Merkle consistency
✅ Issue #5: Duplicate commitment race

Only Issue #6 (test coverage) remains.
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
**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
Issue #4: Current verification_key_merkle.json has identical vk_gamma_2 and
vk_delta_2, indicating unsafe trusted setup that compromises proof security.

This commit adds:

1. REGENERATE_ZK_KEYS_PRODUCTION.md
   - Comprehensive step-by-step guide for regenerating production-safe keys
   - CDN upload instructions (SFTP to tcsenpai@discus.sh)
   - Verification checklist and rollback plan
   - Security notes on single-party vs multi-party ceremonies

2. scripts/regenerate_zk_keys.sh
   - Automated regeneration script with safety verification
   - Backs up old key before regeneration
   - Verifies gamma ≠ delta after generation
   - Provides next steps for commit and CDN upload

Usage:
  ./scripts/regenerate_zk_keys.sh

CDN Upload (after running script):
  sftp tcsenpai@discus.sh
  cd /home/tcsenpai/kynesys/caddycdn/files/zk-circuits/v1
  put src/features/zk/circuits/identity_with_merkle_js/identity_with_merkle.wasm
  put src/features/zk/keys/identity_with_merkle_0000.zkey
  put src/features/zk/keys/verification_key_merkle.json
  exit

Related: CodeRabbit Round 6 Issue #4 (CRITICAL SECURITY)

🤖 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
…K keys

Problem:
- Original keys had identical vk_gamma_2 and vk_delta_2 (single-party setup)
- CodeRabbit Issue #4 identified this as CRITICAL security risk
- Simple regeneration reproduced the same issue

Solution:
- Added random contribution phase after initial key generation
- Modified setup-zk.ts to generate _0000.zkey → contribute → _0001.zkey
- Verification key now exported from contributed key (phase 1)
- Automated verification that gamma ≠ delta

Changes:
- src/features/zk/scripts/setup-zk.ts: Add contribution phase with random entropy
- scripts/regenerate_zk_keys.sh: Clean up both _0000 and _0001 keys, update CDN instructions
- REGENERATE_ZK_KEYS_PRODUCTION.md: Document contribution approach and CDN upload process
- src/features/zk/keys/verification_key_merkle.json: NEW production-safe verification key

Security Impact:
- Keys now have distinct gamma and delta values
- Single random contribution improves security over pure single-party setup
- Maintains backward compatibility (verification still works with new key)

CDN Upload Required:
- identity_with_merkle.wasm (recompiled circuit)
- identity_with_merkle_0001.zkey → renamed to identity_with_merkle_final.zkey
- verification_key_merkle.json (NEW with distinct gamma/delta)

🤖 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
Test now checks for identity_with_merkle_0001.zkey instead of _0000.zkey
to align with the new production-safe keys that use contribution phase.

Related: Issue #4 - Production-safe ZK keys

🤖 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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants