Skip to content

Fees development#1

Merged
tcsenpai merged 4 commits intomainfrom
fees_development
Feb 28, 2024
Merged

Fees development#1
tcsenpai merged 4 commits intomainfrom
fees_development

Conversation

@tcsenpai
Copy link
Contributor

@tcsenpai tcsenpai commented Feb 28, 2024

Primitives for fees and for getting payload size

@tcsenpai tcsenpai merged commit c37cc80 into main Feb 28, 2024
@tcsenpai tcsenpai deleted the fees_development branch February 28, 2024 16:24
tcsenpai pushed a commit that referenced this pull request Oct 10, 2025
@claude
docs: Add Storage Programs documentation and fix critical bugs
db614cc 
## 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 21, 2025
fix(ud): add comprehensive enum and timestamp validation
56fe18e 
Issue: Only checked field presence, not valid values. Invalid enum values
or timestamps could cause downstream errors with unclear messages.

Fix:
- Validate signatureType is 'evm' or 'solana'
- Validate network is one of: polygon, base, sonic, ethereum, solana
- Validate registryType is 'UNS' or 'CNS'
- Validate timestamp is valid number or parseable date (>0)
- Provide clear, actionable error messages for each validation failure

Data Integrity: Prevents invalid data from entering system
Developer Experience: Clear error messages aid debugging
Error Prevention: Catches issues before database write

Reviewer Concern #1
tcsenpai pushed a commit that referenced this pull request Nov 8, 2025
@claude
Fix 8 critical and high-priority issues from CodeRabbit PR review
e114000 
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 8, 2025
@claude
Fix CodeRabbit Issue #1: Make blockchain storage mandatory for both p…
24c7a19 
…aths

Enforces consistent audit trail policy across online and offline message delivery.

BEFORE:
- Offline path: Blockchain failures were logged but non-fatal (operation continued)
- Online path: Blockchain failures aborted the operation (fatal)
- Result: Inconsistent audit trail with potential gaps

AFTER:
- Both paths: Blockchain failures abort the operation
- Ensures complete audit trail for all messages
- Consistent error handling and failure behavior

Changes:
- Updated offline path (lines 422-430) to match online path behavior
- Blockchain storage now mandatory for audit trail consistency
- Both paths return error and abort on blockchain failure

Impact:
- Guarantees all delivered messages have blockchain records
- Prevents audit trail gaps from blockchain service interruptions
- Message delivery requires both DB and blockchain success

🤖 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
docs: Mark Issue #1 (Circuit Privacy) as completed in PR_TOFIX.md
0d468f8 
Issue #1 resolved with Poseidon(3) nullifier computation including secret.
All circuit files recompiled and CDN updated.
tcsenpai pushed a commit that referenced this pull request Nov 10, 2025
Update PR_TOFIX: Mark Issue #4 as completed
52d03f7 
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
@claude
Fix CRITICAL race conditions and transaction boundaries
498f7ba 
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
@claude
Fix 6 priority issues from CodeRabbit Round 4 review
ff604be 
**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
@claude
Fix Round 5 CodeRabbit issues: 6 HIGH/MEDIUM priority fixes
bbaa58d 
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
@claude
Fix CRITICAL #1: Replace optimistic locking with pessimistic locking
3abf041 
Replaced the flawed optimistic nullifier marking strategy with proper
pessimistic locking within database transactions.

**Problem Solved:**
- Optimistic marking left dirty data (blockNumber=0, transactionHash="pending_verification")
- Dummy values were NEVER updated after successful verification
- System crashes could orphan nullifiers with dummy values
- No cleanup mechanism existed for "pending_verification" entries

**Solution Implemented:**
1. ProofVerifier.verifyIdentityAttestation now accepts optional EntityManager
2. Uses pessimistic_write lock within transaction when manager provided
3. Nullifiers marked with CORRECT values ONLY after all verification passes
4. Backward compatible fallback for non-transactional callers

**Changes:**
- src/features/zk/proof/ProofVerifier.ts:
  * Added EntityManager parameter for transactional usage
  * Added metadata parameter for correct blockNumber/txHash
  * Transactional path uses pessimistic lock (RECOMMENDED)
  * Non-transactional fallback for backward compatibility

- src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts:
  * Moved verification INSIDE transaction
  * Passes EntityManager and metadata to verifier
  * Removed manual nullifier update code (verification handles it)
  * Handles both simulate and non-simulate paths correctly

**Benefits:**
- No dirty data: nullifiers only marked with correct values
- TOCTOU race condition prevented with pessimistic_write lock
- Atomic verification + marking + points awarding
- Crashes cannot orphan invalid nullifiers
- No cleanup job needed

🤖 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
@claude
Fix Round 6 Issues #1, #3, #6a: Type consistency, IC validation, circ…
0e250c6 
…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>
@hackobi hackobi mentioned this pull request Feb 13, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@tcsenpai