-
Notifications
You must be signed in to change notification settings - Fork 2
L2ps simplified #495
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: testnet
Are you sure you want to change the base?
L2ps simplified #495
Conversation
L2PS Fix: - parallelNetworks.ts:166: Fixed return type mismatch (return [] instead of return) Pre-existing Issues Fixed: - signalingServer.ts:62: Updated mempool import to mempool_v2 - signalingServer.ts:588: Added cryptographic signature for offline messages (integrity verification) - signalingServer.ts:625-627: Moved DB operations outside loop (10x performance improvement) - datasource.ts:39-53: Removed duplicate entities (Mempool, Transactions, GCRTracker) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Replaced biased sort(() => Math.random() - 0.5) with proper Fisher-Yates shuffle. Problem: - Previous shuffle could favor certain validators by 30-40% - Violated transitivity assumptions of sort algorithms - Caused uneven load distribution across validators Solution: - Implemented Fisher-Yates (Knuth) shuffle algorithm - Guarantees truly uniform random distribution (1/n! for each permutation) - O(n) time complexity (faster than sort's O(n log n)) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Critical fixes: - Transactional offline message delivery with error handling - Parallel validator relay with concurrency limit (prevents blocking) High-priority fixes: - Add | null to l2ps_hashes repo type annotation - Fix TypeORM bigint type mismatch in OfflineMessages - Validate nested data access in handleL2PS (2 locations) - Define L2PSHashPayload interface with validation - Reject transactions without block_number Medium-priority fixes: - Add private constructor to L2PSHashService singleton - Remove redundant @Index from L2PSMempool primary key 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Critical fixes (5/5): - L2PSMempool: Add ensureInitialized() guards to prevent null repository crashes - L2PSMempool: Fix timestamp type (bigint → string) to match TypeORM behavior - RelayRetryService: Add 5-second timeout wrapper for validator calls - RelayRetryService: Add cleanup for retryAttempts Map to prevent memory leak - RelayRetryService: Convert sequential processing to parallel (concurrency: 5) High priority fixes (11/13): - RelayRetryService: Add null safety for validator.identity (3 locations) - L2PSMempool: Add block number validation for edge cases - L2PSMempool: Fix duplicate check consistency (use existsByHash method) - L2PSConcurrentSync: Optimize duplicate detection with batched queries - L2PSConcurrentSync: Use addTransaction() for validation instead of direct insert - L2PSHashes: Fix race condition with atomic upsert operation - RelayRetryService: Add validityDataCache eviction to prevent unbounded growth - SignalingServer: Add consistent error handling for blockchain storage - SignalingServer: Add null safety checks for private key access (2 locations) - ParallelNetworks: Add JSON parsing error handling for config files - ParallelNetworks: Add array validation before destructuring All changes pass ESLint with zero errors or warnings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Fix nonce increment timing: Move senderNonces.set() to after successful mempool addition for better error handling - Add defensive rate limiting: Enforce MAX_OFFLINE_MESSAGES_PER_SENDER in storeOfflineMessage method - Update PR_REVIEW_FINAL.md: Document validation results and remaining issues All changes pass ESLint validation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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>
…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>
|
Warning Rate limit exceeded@tcsenpai has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds a large L2PS (Layer 2 Privacy Subnets) and DTR feature set: new entities, mempools, managers, concurrent sync and hash-generation services, a relay-retry background service, NodeCall endpoints, datasource refactor, offline messaging support, and startup/shutdown wiring. All changes are feature additions and docs; no behavioral rollbacks. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant L2PSNode as L2PS Participant
participant DB as L2PSMempool
participant HashSvc as L2PSHashService
participant Validator
Client->>L2PSNode: Submit encrypted transaction
L2PSNode->>DB: L2PSMempool.addTransaction(encrypted)
Note right of DB: stored encrypted-only
loop every 5s
HashSvc->>DB: getHashForL2PS(uid)
HashSvc->>Validator: Relay L2PS hash update (DTR in PROD)
Validator->>Validator: L2PSHashes.updateHash(uid,hash,meta)
end
sequenceDiagram
participant NonValidator
participant Mempool
participant RelayRetry as RelayRetryService
participant Validator
NonValidator->>Mempool: Receive tx + store validityData
loop every 10s (retry)
RelayRetry->>Validator: Relay txs (shuffled list)
alt validator accepts
Validator->>Mempool: add tx to validator mempool
RelayRetry->>Mempool: removeTransaction(txHash)
RelayRetry->>RelayRetry: clear validityDataCache entry
else all fail / exhausted
RelayRetry->>Mempool: remove tx + abandon or log
end
end
sequenceDiagram
participant Peer
participant Sync as Sync.ts
participant L2PSCS as L2PSConcurrentSync
Peer->>Sync: peer connected / mergePeerlist()
Sync->>L2PSCS: exchangeL2PSParticipation(peers) (non-blocking)
par blockchain sync
Sync->>Sync: requestBlocks()
and L2PS sync (background)
L2PSCS->>Peer: discoverL2PSParticipants()/syncL2PSWithPeer()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes Areas to focus during review:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||||||||
|
This PR modifies Files that will be reverted:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 16
🧹 Nitpick comments (6)
src/features/multichain/routines/executors/pay.ts (1)
129-131: Maintain semicolon consistency with the rest of the file.Lines 129 and 131 remove trailing semicolons, while the rest of the codebase (including this file) consistently uses them. This creates a stylistic inconsistency within the same function and module.
Apply this diff to restore consistency:
- let signedTx = operation.task.signedPayloads[0] + let signedTx = operation.task.signedPayloads[0]; - signedTx = validateIfUint8Array(signedTx) + signedTx = validateIfUint8Array(signedTx);src/model/entities/GCRv2/GCRSubnetsTxs.ts (1)
2-2: Unused import:Transactionis not used in this file.The
Transactiontype is imported but never referenced in the file. Consider removing it to keep imports clean.Apply this diff to remove the unused import:
-import type { L2PSTransaction, Transaction } from "@kynesyslabs/demosdk/types" +import type { L2PSTransaction } from "@kynesyslabs/demosdk/types"src/index.ts (1)
396-416: Add consistent error handling for RelayRetryService startup.The L2PS service startup is wrapped in try-catch (lines 407-413), but the DTR service startup (lines 398-402) lacks error handling. Both services should have consistent error handling for robustness.
Apply this diff:
// Start DTR relay retry service after background loop initialization // The service will wait for syncStatus to be true before actually processing if (getSharedState.PROD) { - console.log("[DTR] Initializing relay retry service (will start after sync)") - // Service will check syncStatus internally before processing - RelayRetryService.getInstance().start() + try { + console.log("[DTR] Initializing relay retry service (will start after sync)") + // Service will check syncStatus internally before processing + RelayRetryService.getInstance().start() + } catch (error) { + console.error("[DTR] Failed to start relay retry service:", error) + } }src/libs/blockchain/routines/Sync.ts (1)
116-130: LGTM! Non-blocking L2PS participant discovery correctly implemented.The background discovery pattern is well-designed:
- Fire-and-forget execution (no
await) ensures blockchain sync isn't blocked- Error isolation prevents L2PS failures from breaking sync
- Clear logging for debugging
Optional: Simplify optional chaining for consistency.
The onboarding guide (
.serena/memories/l2ps_onboarding_guide.md:314) notes thatl2psJoinedUidsis always defined with a default of[]. The optional chaining (?.) is safe but technically redundant. Consider using direct property access for consistency:- if (getSharedState.l2psJoinedUids?.length > 0) { + if (getSharedState.l2psJoinedUids.length > 0) {This same pattern appears at lines 385 and 511 as well.
.serena/memories/l2ps_overview.md (1)
21-31: Optional: Add language specifier to code fence for better rendering.The transaction flow diagram would benefit from a language specifier for syntax highlighting and proper rendering in documentation viewers.
-``` +```text Client → L2PS Node → Decrypt → L2PS Mempool (encrypted storage) ↓src/model/entities/L2PSMempool.ts (1)
27-31: Define composite indexes at entity scope.TypeORM only materializes composite indexes when the decorator sits at the entity/class level. Attaching
@Index(["l2ps_uid", "timestamp"])to the property silently collapses into another single-column index onl2ps_uid, so the intended covering indexes fortimestamp,status, andblock_numbernever land in the schema. Move those composite definitions to the class to get the access paths you’re expecting.-@Entity("l2ps_mempool") +@Index("idx_l2ps_uid_timestamp", ["l2ps_uid", "timestamp"]) +@Index("idx_l2ps_uid_status", ["l2ps_uid", "status"]) +@Index("idx_l2ps_uid_block_number", ["l2ps_uid", "block_number"]) +@Entity("l2ps_mempool") export class L2PSMempoolTx { @@ - @Index() - @Index(["l2ps_uid", "timestamp"]) - @Index(["l2ps_uid", "status"]) - @Index(["l2ps_uid", "block_number"]) + @Index() @Column("text") l2ps_uid: string
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (53)
.gitignore(1 hunks).serena/.gitignore(1 hunks).serena/memories/code_style_conventions.md(1 hunks).serena/memories/codebase_structure.md(1 hunks).serena/memories/development_guidelines.md(1 hunks).serena/memories/l2ps_architecture.md(1 hunks).serena/memories/l2ps_code_patterns.md(1 hunks).serena/memories/l2ps_implementation_status.md(1 hunks).serena/memories/l2ps_onboarding_guide.md(1 hunks).serena/memories/l2ps_overview.md(1 hunks).serena/memories/l2ps_remaining_work.md(1 hunks).serena/memories/project_purpose.md(1 hunks).serena/memories/session_2025_01_31_l2ps_completion.md(1 hunks).serena/memories/suggested_commands.md(1 hunks).serena/memories/task_completion_checklist.md(1 hunks).serena/memories/tech_stack.md(1 hunks).serena/project.yml(1 hunks).vscode/extensions.json(1 hunks).vscode/settings.json(1 hunks)L2PS_PHASES.md(1 hunks)L2PS_TESTING.md(1 hunks)dtr_implementation/DTR_MINIMAL_IMPLEMENTATION.md(1 hunks)dtr_implementation/README.md(1 hunks)dtr_implementation/validator_status_minimal.md(1 hunks)package.json(2 hunks)src/features/InstantMessagingProtocol/signalingServer/plan_of_action_for_offline_messages.md(1 hunks)src/features/InstantMessagingProtocol/signalingServer/signalingServer.ts(10 hunks)src/features/multichain/routines/executors/pay.ts(1 hunks)src/index.ts(2 hunks)src/libs/blockchain/l2ps_hashes.ts(1 hunks)src/libs/blockchain/l2ps_mempool.ts(1 hunks)src/libs/blockchain/mempool_v2.ts(1 hunks)src/libs/blockchain/routines/Sync.ts(4 hunks)src/libs/blockchain/transaction.ts(2 hunks)src/libs/consensus/v2/routines/isValidator.ts(1 hunks)src/libs/l2ps/L2PSConcurrentSync.ts(1 hunks)src/libs/l2ps/L2PSHashService.ts(1 hunks)src/libs/l2ps/L2PS_DTR_IMPLEMENTATION.md(1 hunks)src/libs/l2ps/parallelNetworks.ts(1 hunks)src/libs/network/dtr/relayRetryService.ts(1 hunks)src/libs/network/endpointHandlers.ts(8 hunks)src/libs/network/manageExecution.ts(0 hunks)src/libs/network/manageNodeCall.ts(3 hunks)src/libs/network/routines/transactions/demosWork/handleStep.ts(1 hunks)src/libs/network/routines/transactions/handleL2PS.ts(2 hunks)src/libs/network/server_rpc.ts(1 hunks)src/model/datasource.ts(1 hunks)src/model/entities/GCRv2/GCRSubnetsTxs.ts(2 hunks)src/model/entities/L2PSHashes.ts(1 hunks)src/model/entities/L2PSMempool.ts(1 hunks)src/model/entities/OfflineMessages.ts(1 hunks)src/utilities/sharedState.ts(3 hunks)src/utilities/validateUint8Array.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/libs/network/manageExecution.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T22:57:18.166Z
Learnt from: tcsenpai
Repo: kynesyslabs/node PR: 477
File: .serena/memories/storage_programs_complete.md:64-82
Timestamp: 2025-10-10T22:57:18.166Z
Learning: Files in the `.serena/memories/` directory should be left alone - do not suggest linting fixes, formatting changes, or other modifications to files in this directory.
Applied to files:
.serena/memories/project_purpose.md.serena/memories/task_completion_checklist.md.serena/memories/development_guidelines.md.serena/.gitignore.serena/memories/suggested_commands.md.serena/memories/code_style_conventions.md.serena/project.yml.serena/memories/codebase_structure.md
🧬 Code graph analysis (18)
src/libs/blockchain/mempool_v2.ts (1)
src/utilities/logger.ts (1)
error(125-132)
src/libs/consensus/v2/routines/isValidator.ts (2)
src/libs/consensus/v2/routines/getCommonValidatorSeed.ts (1)
getCommonValidatorSeed(58-132)src/utilities/sharedState.ts (1)
getSharedState(238-240)
src/index.ts (4)
src/utilities/sharedState.ts (1)
getSharedState(238-240)src/libs/network/dtr/relayRetryService.ts (1)
RelayRetryService(22-343)src/libs/l2ps/L2PSHashService.ts (1)
L2PSHashService(24-410)src/utilities/logger.ts (1)
error(125-132)
src/libs/blockchain/routines/Sync.ts (3)
src/utilities/sharedState.ts (1)
getSharedState(238-240)src/libs/l2ps/L2PSConcurrentSync.ts (3)
discoverL2PSParticipants(30-88)syncL2PSWithPeer(110-246)exchangeL2PSParticipation(267-303)src/libs/peer/Peer.ts (1)
Peer(15-346)
src/model/entities/OfflineMessages.ts (1)
src/model/entities/L2PSMempool.ts (1)
Entity(13-72)
src/model/entities/L2PSMempool.ts (1)
src/model/entities/OfflineMessages.ts (1)
Entity(4-34)
src/libs/l2ps/L2PSHashService.ts (3)
src/utilities/sharedState.ts (2)
SharedState(17-233)getSharedState(238-240)src/libs/blockchain/l2ps_mempool.ts (1)
L2PSMempool(25-474)src/libs/consensus/v2/routines/getCommonValidatorSeed.ts (1)
getCommonValidatorSeed(58-132)
src/libs/network/server_rpc.ts (1)
src/utilities/sharedState.ts (1)
getSharedState(238-240)
src/libs/network/endpointHandlers.ts (8)
src/utilities/sharedState.ts (1)
getSharedState(238-240)src/libs/consensus/v2/routines/isValidator.ts (1)
isValidatorForNextBlock(6-15)src/libs/consensus/v2/routines/getCommonValidatorSeed.ts (1)
getCommonValidatorSeed(58-132)src/libs/consensus/v2/routines/getShard.ts (1)
getShard(8-84)src/utilities/logger.ts (1)
error(125-132)src/libs/network/server_rpc.ts (1)
emptyResponse(35-40)src/libs/l2ps/parallelNetworks.ts (1)
ParallelNetworks(61-425)src/libs/blockchain/l2ps_hashes.ts (1)
L2PSHashes(22-234)
src/libs/blockchain/l2ps_hashes.ts (1)
src/utilities/logger.ts (1)
error(125-132)
src/features/multichain/routines/executors/pay.ts (1)
src/utilities/validateUint8Array.ts (1)
validateIfUint8Array(1-9)
src/libs/blockchain/l2ps_mempool.ts (2)
src/utilities/logger.ts (1)
error(125-132)src/libs/consensus/v2/types/secretaryManager.ts (1)
SecretaryManager(15-907)
src/libs/network/dtr/relayRetryService.ts (5)
src/libs/blockchain/mempool_v2.ts (1)
Mempool(11-222)src/utilities/sharedState.ts (1)
getSharedState(238-240)src/utilities/logger.ts (1)
error(125-132)src/libs/consensus/v2/routines/isValidator.ts (1)
isValidatorForNextBlock(6-15)src/libs/consensus/v2/routines/getCommonValidatorSeed.ts (1)
getCommonValidatorSeed(58-132)
src/features/InstantMessagingProtocol/signalingServer/signalingServer.ts (3)
src/libs/blockchain/transaction.ts (1)
Transaction(50-540)src/utilities/sharedState.ts (1)
getSharedState(238-240)src/libs/blockchain/mempool_v2.ts (1)
Mempool(11-222)
src/libs/network/manageNodeCall.ts (5)
src/utilities/sharedState.ts (1)
getSharedState(238-240)src/libs/consensus/v2/routines/isValidator.ts (1)
isValidatorForNextBlock(6-15)src/libs/blockchain/transaction.ts (2)
Transaction(50-540)isCoherent(277-287)src/libs/blockchain/mempool_v2.ts (1)
Mempool(11-222)src/libs/blockchain/l2ps_mempool.ts (1)
L2PSMempool(25-474)
src/libs/l2ps/parallelNetworks.ts (2)
src/utilities/sharedState.ts (1)
getSharedState(238-240)src/libs/blockchain/transaction.ts (1)
Transaction(50-540)
src/libs/network/routines/transactions/handleL2PS.ts (4)
src/libs/network/server_rpc.ts (1)
emptyResponse(35-40)src/libs/l2ps/parallelNetworks.ts (1)
ParallelNetworks(61-425)src/libs/blockchain/transaction.ts (1)
Transaction(50-540)src/libs/blockchain/l2ps_mempool.ts (1)
L2PSMempool(25-474)
src/libs/l2ps/L2PSConcurrentSync.ts (3)
src/libs/peer/Peer.ts (1)
Peer(15-346)src/utilities/logger.ts (1)
error(125-132)src/libs/blockchain/l2ps_mempool.ts (1)
L2PSMempool(25-474)
🪛 Biome (2.1.2)
src/libs/network/endpointHandlers.ts
[error] 401-402: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 LanguageTool
dtr_implementation/README.md
[grammar] ~252-~252: Use a hyphen to join words.
Context: ... relay optimization - Quality-of-service based routing ### **Phase 3: Incentive ...
(QB_NEW_EN_HYPHEN)
.serena/memories/l2ps_onboarding_guide.md
[style] ~363-~363: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... generation?** → L2PSHashService.ts Need to see sync logic? → `L2PSConcurrentSync...
(REP_NEED_TO_VB)
[style] ~364-~364: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...nc logic?** → L2PSConcurrentSync.ts Need to see endpoints? → manageNodeCall.ts ...
(REP_NEED_TO_VB)
[style] ~365-~365: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...→ manageNodeCall.ts (lines 318-421) Need to see blockchain integration? → `Sync.t...
(REP_NEED_TO_VB)
[style] ~366-~366: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...on?** → Sync.ts (search for "L2PS") Need to understand storage? → `l2ps_mempool.t...
(REP_NEED_TO_VB)
L2PS_PHASES.md
[style] ~595-~595: Consider an alternative to avoid wordiness and to strengthen your wording.
Context: ...ck**: Confirm blockchain sync continues working without issues ### Documentation Check - All new code...
(WORKS_WITHOUT_PROBLEMS)
🪛 markdownlint-cli2 (0.18.1)
dtr_implementation/README.md
51-51: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.serena/memories/l2ps_architecture.md
5-5: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
50-50: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
67-67: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
90-90: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
115-115: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
156-156: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
179-179: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
L2PS_TESTING.md
45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
86-86: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
190-190: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
222-222: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
250-250: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
280-280: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
297-297: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
314-314: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.serena/memories/l2ps_onboarding_guide.md
22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
177-177: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
188-188: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
199-199: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.serena/memories/l2ps_overview.md
21-21: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
src/features/InstantMessagingProtocol/signalingServer/plan_of_action_for_offline_messages.md
14-14: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
130-130: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
279-279: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
295-295: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
303-303: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
310-310: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
343-343: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
dtr_implementation/DTR_MINIMAL_IMPLEMENTATION.md
45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
86-86: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
190-190: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
222-222: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
250-250: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
280-280: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
297-297: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
314-314: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.serena/memories/codebase_structure.md
6-6: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
41-41: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
55-55: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
75-75: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (11)
.vscode/extensions.json (1)
33-34: JSON syntax is valid and the extension addition is appropriate.The trailing comma added to line 33 is correct for valid JSON, and the new VS Code extension recommendation is properly formatted as the final array element.
.vscode/settings.json (1)
1-4: Verify that the configuration reduction is intentional and doesn't impact team workflows.This change significantly reduces the workspace configuration from a larger set of settings (including formatters, visualization options, and dimmer features per the summary) down to just two UI preferences. While simplification can be beneficial, this substantial reduction warrants verification.
Please confirm:
- Was this intentional as part of the L2PS simplification effort, or should some settings be preserved?
- Are critical development settings (formatters, linters, extensions configuration) managed elsewhere in the project?
- Should the team be notified of this change in case they have local overrides they depend on?
src/model/entities/GCRv2/GCRSubnetsTxs.ts (1)
26-27: No breaking changes from type conversion.The type change from
EncryptedTransactiontoL2PSTransactionis safe. EncryptedTransaction does not exist in the codebase, and tx_data is never accessed for property reads—the field is treated as an opaque JSON column by TypeORM. The only mutation is setting it to null, which is compatible with both types.Likely an incorrect or invalid review comment.
.serena/project.yml (1)
1-84: Serena project configuration looks appropriate.Standard configuration for TypeScript project with proper tooling defaults, gitignore respect enabled, and no overly restrictive tool exclusions. The project setup aligns with the codebase structure and development guidelines documented elsewhere.
src/utilities/sharedState.ts (1)
57-59: LGTM - Well-designed additions for DTR and L2PS features.The new
validityDataCacheMap andl2psJoinedUidsarray are appropriately scoped and documented. The cache supports the DTR relay retry mechanism, and cleanup logic exists inRelayRetryServiceto prevent memory leaks.Also applies to: 85-86
src/libs/network/routines/transactions/demosWork/handleStep.ts (1)
11-11: Missing.jsextension in ES module import.The import path should include the
.jsextension for proper ESM resolution.Apply this diff:
-import { L2PSMessage } from "@/libs/l2ps/parallelNetworks_deprecated" +import { L2PSMessage } from "@/libs/l2ps/parallelNetworks_deprecated.js"Note: The
_deprecatedsuffix suggests this module may need future refactoring or removal.Likely an incorrect or invalid review comment.
src/libs/consensus/v2/routines/isValidator.ts (1)
1-3: Missing.jsextensions in ES module imports.All three imports should include
.jsextensions for proper ESM module resolution.Apply this diff:
-import getShard from "./getShard" -import getCommonValidatorSeed from "./getCommonValidatorSeed" -import { getSharedState } from "@/utilities/sharedState" +import getShard from "./getShard.js" +import getCommonValidatorSeed from "./getCommonValidatorSeed.js" +import { getSharedState } from "@/utilities/sharedState.js"Likely an incorrect or invalid review comment.
src/libs/blockchain/routines/Sync.ts (2)
30-34: LGTM! Clean L2PS integration imports.The imports are well-organized and align with the non-blocking L2PS integration hooks added below.
383-396: LGTM! L2PS mempool sync correctly integrated without blocking blockchain operations.The implementation demonstrates excellent separation of concerns:
- Runs after successful block download as a background task
- Per-UID synchronization ensures granular error handling
- Explicit comment reinforces the critical design principle: "Don't break blockchain sync on L2PS errors"
src/model/entities/L2PSHashes.ts (1)
1-55: LGTM! Well-designed entity for validator hash storage.The entity implementation demonstrates strong attention to detail:
- Data integrity: Non-null constraints on critical fields (hash, transaction_count)
- Type safety: Proper handling of bigint columns as strings to match TypeORM runtime behavior
- Documentation: Clear JSDoc explaining the privacy model and field purposes
- Schema design: Primary key on
l2ps_uidensures one hash mapping per L2PS networkThe design correctly supports the content-blind validator consensus model described in the documentation.
src/model/entities/OfflineMessages.ts (1)
1-34: LGTM! Well-structured entity for offline messaging.The entity design is solid with appropriate considerations:
- Indexing strategy: Efficient lookups by recipient and sender keys for message retrieval
- Uniqueness: Message hash uniqueness prevents duplicate storage
- Type safety: Proper bigint-as-string handling consistent with TypeORM behavior
- Status semantics: The "sent" status accurately reflects websocket semantics (no delivery guarantee)
The schema aligns well with the offline messaging workflow described in the PR documentation.
| "scripts": { | ||
| "lint": "prettier --plugin-search-dir . --check . && eslint .", | ||
| "lint:fix": "eslint . --fix --ext .ts", | ||
| "lint:fix": "eslint . --fix --ext .ts --ignore-pattern 'local_tests/**'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Windows-safe quoting for --ignore-pattern.
npm run executes through cmd.exe on Windows, where single quotes are treated as literal characters. As written, the ignore pattern becomes 'local_tests/**' and never matches, so Windows contributors still lint local_tests/** despite this change. Switch to escaped double quotes so the pattern is honored cross-platform.(archive.eslint.org)
- "lint:fix": "eslint . --fix --ext .ts --ignore-pattern 'local_tests/**'",
+ "lint:fix": "eslint . --fix --ext .ts --ignore-pattern \"local_tests/**\"",🤖 Prompt for AI Agents
In package.json at line 11, the lint:fix script uses single quotes around
--ignore-pattern ('local_tests/**') which are treated literally on Windows;
replace the single quotes with escaped double quotes (i.e. \"local_tests/**\")
so the ignore pattern is honored cross-platform (update the script value
accordingly in package.json).
src/features/InstantMessagingProtocol/signalingServer/signalingServer.ts
Show resolved
Hide resolved
| // Graceful shutdown handling for DTR service | ||
| process.on("SIGINT", () => { | ||
| console.log("[Services] Received SIGINT, shutting down gracefully...") | ||
| if (getSharedState.PROD) { | ||
| RelayRetryService.getInstance().stop() | ||
| } | ||
|
|
||
| // Stop L2PS hash service if running | ||
| try { | ||
| L2PSHashService.getInstance().stop() | ||
| } catch (error) { | ||
| console.error("[L2PS] Error stopping hash service:", error) | ||
| } | ||
|
|
||
| process.exit(0) | ||
| }) | ||
|
|
||
| process.on("SIGTERM", () => { | ||
| console.log("[Services] Received SIGTERM, shutting down gracefully...") | ||
| if (getSharedState.PROD) { | ||
| RelayRetryService.getInstance().stop() | ||
| } | ||
|
|
||
| // Stop L2PS hash service if running | ||
| try { | ||
| L2PSHashService.getInstance().stop() | ||
| } catch (error) { | ||
| console.error("[L2PS] Error stopping hash service:", error) | ||
| } | ||
|
|
||
| process.exit(0) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Eliminate duplicate shutdown logic.
The SIGINT and SIGTERM handlers contain identical shutdown code, violating the DRY principle and making maintenance harder.
Apply this diff to extract common shutdown logic:
+// Graceful shutdown function
+async function gracefulShutdown(signal: string): Promise<void> {
+ console.log(`[Services] Received ${signal}, shutting down gracefully...`)
+
+ if (getSharedState.PROD) {
+ RelayRetryService.getInstance().stop()
+ }
+
+ // Stop L2PS hash service if running
+ try {
+ await L2PSHashService.getInstance().stop()
+ } catch (error) {
+ console.error("[L2PS] Error stopping hash service:", error)
+ }
+
+ process.exit(0)
+}
+
-// Graceful shutdown handling for DTR service
-process.on("SIGINT", () => {
- console.log("[Services] Received SIGINT, shutting down gracefully...")
- if (getSharedState.PROD) {
- RelayRetryService.getInstance().stop()
- }
-
- // Stop L2PS hash service if running
- try {
- L2PSHashService.getInstance().stop()
- } catch (error) {
- console.error("[L2PS] Error stopping hash service:", error)
- }
-
- process.exit(0)
-})
-
-process.on("SIGTERM", () => {
- console.log("[Services] Received SIGTERM, shutting down gracefully...")
- if (getSharedState.PROD) {
- RelayRetryService.getInstance().stop()
- }
-
- // Stop L2PS hash service if running
- try {
- L2PSHashService.getInstance().stop()
- } catch (error) {
- console.error("[L2PS] Error stopping hash service:", error)
- }
-
- process.exit(0)
-})
+process.on("SIGINT", () => gracefulShutdown("SIGINT"))
+process.on("SIGTERM", () => gracefulShutdown("SIGTERM"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Graceful shutdown handling for DTR service | |
| process.on("SIGINT", () => { | |
| console.log("[Services] Received SIGINT, shutting down gracefully...") | |
| if (getSharedState.PROD) { | |
| RelayRetryService.getInstance().stop() | |
| } | |
| // Stop L2PS hash service if running | |
| try { | |
| L2PSHashService.getInstance().stop() | |
| } catch (error) { | |
| console.error("[L2PS] Error stopping hash service:", error) | |
| } | |
| process.exit(0) | |
| }) | |
| process.on("SIGTERM", () => { | |
| console.log("[Services] Received SIGTERM, shutting down gracefully...") | |
| if (getSharedState.PROD) { | |
| RelayRetryService.getInstance().stop() | |
| } | |
| // Stop L2PS hash service if running | |
| try { | |
| L2PSHashService.getInstance().stop() | |
| } catch (error) { | |
| console.error("[L2PS] Error stopping hash service:", error) | |
| } | |
| process.exit(0) | |
| }) | |
| // Graceful shutdown function | |
| async function gracefulShutdown(signal: string): Promise<void> { | |
| console.log(`[Services] Received ${signal}, shutting down gracefully...`) | |
| if (getSharedState.PROD) { | |
| RelayRetryService.getInstance().stop() | |
| } | |
| // Stop L2PS hash service if running | |
| try { | |
| await L2PSHashService.getInstance().stop() | |
| } catch (error) { | |
| console.error("[L2PS] Error stopping hash service:", error) | |
| } | |
| process.exit(0) | |
| } | |
| process.on("SIGINT", () => gracefulShutdown("SIGINT")) | |
| process.on("SIGTERM", () => gracefulShutdown("SIGTERM")) |
🤖 Prompt for AI Agents
In src/index.ts around lines 420 to 451, the SIGINT and SIGTERM handlers
duplicate the same shutdown logic; extract that logic into a single shared
function (e.g., handleShutdown or gracefulShutdown) that performs logging,
conditionally stops RelayRetryService when getSharedState.PROD, stops
L2PSHashService inside a try/catch, and exits the process, then register that
function as the callback for both process.on("SIGINT", ...) and
process.on("SIGTERM", ...). Ensure the shared function preserves existing
console messages, error logging, and calls process.exit(0) at the end.
| log.error("[L2PS Hashes] Failed to initialize:", error) | ||
| throw error | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Log the actual error details
log.error("[L2PS Hashes] Failed to initialize:", error) is passing the error object into the logToTerminal boolean slot (see src/utilities/logger.ts). As a result the stack/message never reaches the logs, which makes diagnosing init failures very hard. Please stringify/embed the error into the message (same applies to the other log.error calls in this class).
- log.error("[L2PS Hashes] Failed to initialize:", error)
+ const details = error instanceof Error ? error.stack ?? error.message : JSON.stringify(error)
+ log.error(`[L2PS Hashes] Failed to initialize: ${details}`)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| log.error("[L2PS Hashes] Failed to initialize:", error) | |
| throw error | |
| } | |
| const details = error instanceof Error ? error.stack ?? error.message : JSON.stringify(error) | |
| log.error(`[L2PS Hashes] Failed to initialize: ${details}`) | |
| throw error | |
| } |
🤖 Prompt for AI Agents
In src/libs/blockchain/l2ps_hashes.ts around lines 39-41, the call
log.error("[L2PS Hashes] Failed to initialize:", error) is passing the Error
object into the boolean slot of logToTerminal so the stack/message is not
logged; replace it by embedding/stringifying the error into the message (e.g.
build a string using error instanceof Error ? (error.stack || error.message) :
JSON.stringify(error)) and call log.error with that single string, and apply the
same stringifying/embed pattern to the other log.error calls in this class so
the actual error details are logged.
| log.error("[L2PS Mempool] Error adding transaction:", error) | ||
| return { | ||
| success: false, | ||
| error: error.message || "Unknown error", | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve stack traces in error logs
Here (and elsewhere in this class) we call log.error("[L2PS Mempool] Error adding transaction:", error). Because logger.error only accepts (message: string, logToTerminal?: boolean), the error object is swallowed—we just set the terminal flag and lose the stack/message. That makes production incidents opaque. Please embed the error details in the string (and update the other log.error invocations here accordingly).
- log.error("[L2PS Mempool] Error adding transaction:", error)
+ const details = error instanceof Error ? error.stack ?? error.message : JSON.stringify(error)
+ log.error(`[L2PS Mempool] Error adding transaction: ${details}`)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| log.error("[L2PS Mempool] Error adding transaction:", error) | |
| return { | |
| success: false, | |
| error: error.message || "Unknown error", | |
| } | |
| const details = error instanceof Error ? error.stack ?? error.message : JSON.stringify(error) | |
| log.error(`[L2PS Mempool] Error adding transaction: ${details}`) | |
| return { | |
| success: false, | |
| error: error.message || "Unknown error", | |
| } |
🤖 Prompt for AI Agents
In src/libs/blockchain/l2ps_mempool.ts around lines 163 to 167, the logger call
passes the Error object as a second argument and thus loses the stack/message
because logger.error only accepts (message: string, logToTerminal?: boolean);
change the call to interpolate the error details into the message string (for
example include error.stack || error.message || String(error)) and remove the
second argument; apply the same pattern to other log.error invocations in this
class so every error log includes the error stack/message in the formatted
string rather than passing the error object as a separate parameter.
| if (!tx.block_number) { | ||
| response.result = 400 | ||
| response.response = "Missing block_number" | ||
| response.extra = "L2PS hash updates require valid block_number (cannot default to 0)" | ||
| return response | ||
| } | ||
|
|
||
| const payloadData = tx.content.data[1] | ||
|
|
||
| // Validate payload has required L2PSHashPayload structure | ||
| if ( | ||
| typeof payloadData !== "object" || | ||
| !("l2ps_uid" in payloadData) || | ||
| !("consolidated_hash" in payloadData) || | ||
| !("transaction_count" in payloadData) | ||
| ) { | ||
| response.result = 400 | ||
| response.response = "Invalid L2PS hash payload" | ||
| response.extra = "Missing required fields: l2ps_uid, consolidated_hash, or transaction_count" | ||
| return response | ||
| } | ||
|
|
||
| // Extract L2PS hash payload from transaction data with proper typing | ||
| const l2psHashPayload = payloadData as L2PSHashPayload | ||
| const l2psUid = l2psHashPayload.l2ps_uid | ||
|
|
||
| // Validate sender is part of the L2PS network | ||
| const parallelNetworks = ParallelNetworks.getInstance() | ||
| const l2psInstance = await parallelNetworks.getL2PS(l2psUid) | ||
|
|
||
| if (!l2psInstance) { | ||
| response.result = 403 | ||
| response.response = "Not participant in L2PS network" | ||
| response.extra = `L2PS network ${l2psUid} not found or not joined` | ||
| return response | ||
| } | ||
|
|
||
| // REVIEW: Store hash update for validator consensus (Phase 3b) | ||
| // Validators store ONLY UID → hash mappings (content blind) | ||
| try { | ||
| await L2PSHashes.updateHash( | ||
| l2psHashPayload.l2ps_uid, | ||
| l2psHashPayload.consolidated_hash, | ||
| l2psHashPayload.transaction_count, | ||
| BigInt(tx.block_number), // Now guaranteed to exist due to validation above | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix blockNumber validation to avoid rejecting every hash update.
Transaction instances expose blockNumber (camelCase). Checking tx.block_number always yields undefined, so this branch fires for every legitimate hash update and you return 400, preventing validators from persisting any L2PS hash updates. The falsy check also blocks block height 0 if it ever appears. Guard against undefined/null on the camelCase property and use it when persisting.
- if (!tx.block_number) {
- response.result = 400
- response.response = "Missing block_number"
- response.extra = "L2PS hash updates require valid block_number (cannot default to 0)"
+ if (tx.blockNumber === undefined || tx.blockNumber === null) {
+ response.result = 400
+ response.response = "Missing blockNumber"
+ response.extra = "L2PS hash updates require a valid blockNumber"
return response
}
…
- BigInt(tx.block_number), // Now guaranteed to exist due to validation above
+ BigInt(tx.blockNumber), // Now guaranteed to exist due to validation above📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!tx.block_number) { | |
| response.result = 400 | |
| response.response = "Missing block_number" | |
| response.extra = "L2PS hash updates require valid block_number (cannot default to 0)" | |
| return response | |
| } | |
| const payloadData = tx.content.data[1] | |
| // Validate payload has required L2PSHashPayload structure | |
| if ( | |
| typeof payloadData !== "object" || | |
| !("l2ps_uid" in payloadData) || | |
| !("consolidated_hash" in payloadData) || | |
| !("transaction_count" in payloadData) | |
| ) { | |
| response.result = 400 | |
| response.response = "Invalid L2PS hash payload" | |
| response.extra = "Missing required fields: l2ps_uid, consolidated_hash, or transaction_count" | |
| return response | |
| } | |
| // Extract L2PS hash payload from transaction data with proper typing | |
| const l2psHashPayload = payloadData as L2PSHashPayload | |
| const l2psUid = l2psHashPayload.l2ps_uid | |
| // Validate sender is part of the L2PS network | |
| const parallelNetworks = ParallelNetworks.getInstance() | |
| const l2psInstance = await parallelNetworks.getL2PS(l2psUid) | |
| if (!l2psInstance) { | |
| response.result = 403 | |
| response.response = "Not participant in L2PS network" | |
| response.extra = `L2PS network ${l2psUid} not found or not joined` | |
| return response | |
| } | |
| // REVIEW: Store hash update for validator consensus (Phase 3b) | |
| // Validators store ONLY UID → hash mappings (content blind) | |
| try { | |
| await L2PSHashes.updateHash( | |
| l2psHashPayload.l2ps_uid, | |
| l2psHashPayload.consolidated_hash, | |
| l2psHashPayload.transaction_count, | |
| BigInt(tx.block_number), // Now guaranteed to exist due to validation above | |
| ) | |
| if (tx.blockNumber === undefined || tx.blockNumber === null) { | |
| response.result = 400 | |
| response.response = "Missing blockNumber" | |
| response.extra = "L2PS hash updates require a valid blockNumber" | |
| return response | |
| } | |
| const payloadData = tx.content.data[1] | |
| // Validate payload has required L2PSHashPayload structure | |
| if ( | |
| typeof payloadData !== "object" || | |
| !("l2ps_uid" in payloadData) || | |
| !("consolidated_hash" in payloadData) || | |
| !("transaction_count" in payloadData) | |
| ) { | |
| response.result = 400 | |
| response.response = "Invalid L2PS hash payload" | |
| response.extra = "Missing required fields: l2ps_uid, consolidated_hash, or transaction_count" | |
| return response | |
| } | |
| // Extract L2PS hash payload from transaction data with proper typing | |
| const l2psHashPayload = payloadData as L2PSHashPayload | |
| const l2psUid = l2psHashPayload.l2ps_uid | |
| // Validate sender is part of the L2PS network | |
| const parallelNetworks = ParallelNetworks.getInstance() | |
| const l2psInstance = await parallelNetworks.getL2PS(l2psUid) | |
| if (!l2psInstance) { | |
| response.result = 403 | |
| response.response = "Not participant in L2PS network" | |
| response.extra = `L2PS network ${l2psUid} not found or not joined` | |
| return response | |
| } | |
| // REVIEW: Store hash update for validator consensus (Phase 3b) | |
| // Validators store ONLY UID → hash mappings (content blind) | |
| try { | |
| await L2PSHashes.updateHash( | |
| l2psHashPayload.l2ps_uid, | |
| l2psHashPayload.consolidated_hash, | |
| l2psHashPayload.transaction_count, | |
| BigInt(tx.blockNumber), // Now guaranteed to exist due to validation above | |
| ) |
🤖 Prompt for AI Agents
In src/libs/network/endpointHandlers.ts around lines 772 to 817, the code checks
tx.block_number which is always undefined (Transaction exposes camelCase
blockNumber) and uses a falsy check that would also reject a legitimate block
height of 0; change the validation to explicitly guard against undefined/null on
tx.blockNumber (e.g. if (tx.blockNumber === undefined || tx.blockNumber ===
null) { ... }) and then use tx.blockNumber when persisting
(BigInt(tx.blockNumber)), replacing all tx.block_number references accordingly.
| const signatureValid = TxUtils.validateSignature(transaction) | ||
| if (!signatureValid) { | ||
| log.error("[DTR] Transaction signature validation failed: " + transaction.hash) | ||
| response.result = 400 | ||
| response.response = "Transaction signature validation failed" | ||
| break | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix signature validation for relayed transactions.
TxUtils.validateSignature returns a Promise<{ success: boolean; message: string }>; without awaiting it (and checking success) this branch always treats the relay as valid, so a forged payload walks straight into the validator’s mempool. Await the call and reject whenever success is false (logging the reason) to keep invalid transactions out.
- const signatureValid = TxUtils.validateSignature(transaction)
- if (!signatureValid) {
- log.error("[DTR] Transaction signature validation failed: " + transaction.hash)
- response.result = 400
- response.response = "Transaction signature validation failed"
+ const signatureValidation = await TxUtils.validateSignature(transaction)
+ if (!signatureValidation.success) {
+ log.error(
+ `[DTR] Transaction signature validation failed for ${transaction.hash}: ${signatureValidation.message}`,
+ )
+ response.result = 400
+ response.response =
+ signatureValidation.message ||
+ "Transaction signature validation failed"
break
}🤖 Prompt for AI Agents
In src/libs/network/manageNodeCall.ts around lines 287 to 293, the code calls
TxUtils.validateSignature without awaiting it and treats the return as a
boolean; change this to await TxUtils.validateSignature(transaction), check the
returned object’s success property, and if success is false log the returned
message/reason, set response.result = 400 and response.response to the failure
message, then break to reject the transaction so forged/invalid relayed
transactions are not accepted.
| const verificationResult = await Transaction.confirmTx(decryptedTx, decryptedTx.content.from) | ||
| if (!verificationResult) { | ||
| response.result = 400 | ||
| response.response = false | ||
| response.extra = "Transaction signature verification failed" | ||
| return response | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Respect the validation result from Transaction.confirmTx.
Transaction.confirmTx never throws on signature/coherence failure; it returns { success: false, message, confirmation: null }. Because the code only checks for a falsy value, failed validations still flow to the L2PS mempool. Use the returned success flag (and message) to short‑circuit invalid decrypted transactions.
- const verificationResult = await Transaction.confirmTx(decryptedTx, decryptedTx.content.from)
- if (!verificationResult) {
- response.result = 400
- response.response = false
- response.extra = "Transaction signature verification failed"
- return response
- }
+ const verificationResult = await Transaction.confirmTx(
+ decryptedTx,
+ decryptedTx.content.from,
+ )
+ if (!verificationResult?.success) {
+ response.result = 400
+ response.response = false
+ response.extra =
+ verificationResult?.message ||
+ "Transaction signature verification failed"
+ return response
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const verificationResult = await Transaction.confirmTx(decryptedTx, decryptedTx.content.from) | |
| if (!verificationResult) { | |
| response.result = 400 | |
| response.response = false | |
| response.extra = "Transaction signature verification failed" | |
| return response | |
| } | |
| const verificationResult = await Transaction.confirmTx( | |
| decryptedTx, | |
| decryptedTx.content.from, | |
| ) | |
| if (!verificationResult?.success) { | |
| response.result = 400 | |
| response.response = false | |
| response.extra = | |
| verificationResult?.message || | |
| "Transaction signature verification failed" | |
| return response | |
| } |
🤖 Prompt for AI Agents
In src/libs/network/routines/transactions/handleL2PS.ts around lines 80 to 86,
Transaction.confirmTx returns an object like { success: boolean, message:
string, confirmation: any } and the current code only checks for a falsy value,
letting failed validations proceed; update the logic to inspect
verificationResult.success and when false set response.result to 400,
response.response to false and response.extra to verificationResult.message (or
a fallback message), then return response to short‑circuit invalid decrypted
transactions.
|
This PR modifies Files that will be reverted:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
AGENTS.md (2)
103-108: Use heading syntax for better document structure.Line 103 uses bold emphasis (
**Recommended approach:**) where a proper markdown heading would be more appropriate for navigation and document hierarchy.-**Recommended approach:** -- Create a `history/` directory in the project root +### Recommended Approach + +- Create a `history/` directory in the project root
112-115: Specify language for fenced code block.The
.gitignoreexample lacks a language identifier for syntax highlighting.-**Example .gitignore entry (optional):** -``` +**Example .gitignore entry (optional):** +```gitignore # AI planning documents (ephemeral) history/ -``` +```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.beads/.gitignore(1 hunks).beads/.local_version(1 hunks).beads/config.yaml(1 hunks).beads/metadata.json(1 hunks).gitignore(3 hunks)AGENTS.md(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- .beads/metadata.json
- .beads/.gitignore
- .beads/config.yaml
- .beads/.local_version
🧰 Additional context used
🪛 LanguageTool
AGENTS.md
[uncategorized] ~73-~73: The official name of this software platform is spelled with a capital “H”.
Context: ...n If using GitHub Copilot, also create .github/copilot-instructions.md for automatic ...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
AGENTS.md
103-103: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
112-112: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (2)
AGENTS.md (1)
1-134: LGTM — Well-structured documentation for AI agent workflow.The documentation provides clear, actionable guidance on using bd for centralized issue tracking, establishing consistent workflows, and managing ephemeral planning documents. The emphasis on using bd exclusively (avoiding markdown TODOs) and the
history/directory pattern for AI-generated planning docs aligns well with good project governance practices. The quick-start commands and rules are practical and easy to follow.Before merging, please verify that:
- The
bdtool and all referenced commands (bd ready,bd create,bd update,bd close,bd onboard) are installed and available in this project's development environment- The
.beads/issues.jsonlpath and format match the actual bd configuration- The GitHub Copilot integration guidance aligns with team setup (if Copilot is in use)
.gitignore (1)
193-205: Clarify the purpose of PR review artifact patterns.Lines 193–205 add numerous PR review markdown files to
.gitignore(e.g.,PR_REVIEW.md,BUGS_AND_SECURITY_REPORT.md,ZK_CEREMONY_GUIDE.md,CEREMONY_COORDINATION.md, etc.). These appear to be temporary or one-off analysis artifacts from the review process.Adding specific PR-related ignore patterns to the long-term
.gitignoreis unusual unless these files are expected to persist in local development environments. If these are temporary artifacts that should not be committed, clean them up now rather than adding rules for them. If they are intentional, document their purpose.
| CLAUDE.md | ||
| data/l2ps/example/config.json | ||
| data/l2ps/example/iv.key | ||
| data/l2ps/example/config.json | ||
| data/l2ps/example/iv.key | ||
| data/l2ps/* | ||
|
|
||
| # Claude specific files | ||
| CLAUDE.md | ||
| GEMINI.md | ||
|
|
||
| architecture.gitbook-cache.json | ||
| architecture | ||
| architecture |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove duplicate ignore entries.
Several entries are duplicated, creating redundancy and suggesting the file wasn't carefully reviewed before commit:
- Line 114 (
CLAUDE.md) duplicates line 122 - Lines 115–116 (
data/l2ps/example/*) duplicate lines 117–118 - Line 126 (
architecture) duplicates line 127 - Line 119 (
data/l2ps/*) already covers theexample/subdirectory entries, making lines 115–118 partially redundant
Clean up the duplicates and consolidate patterns where feasible to keep the file maintainable.
Suggested cleanup:
# L2PS files
-CLAUDE.md
-data/l2ps/example/config.json
-data/l2ps/example/iv.key
data/l2ps/example/config.json
data/l2ps/example/iv.key
data/l2ps/*
# Claude specific files
CLAUDE.md
-architecture
architectureOr, if the broader pattern data/l2ps/* is preferred, remove the more specific example/ entries entirely.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .gitignore around lines 114 to 127, there are duplicate and redundant entries
(CLAUDE.md repeated, data/l2ps/example/config.json and iv.key duplicated,
data/l2ps/* already covers the example/ entries, and architecture repeated);
remove the duplicated lines and consolidate by keeping a single CLAUDE.md,
remove the specific data/l2ps/example/* lines if you keep data/l2ps/* (or vice
versa), and keep only one architecture entry so each ignore pattern appears
once.
|




User description
L2PS Reworking
PR Type
Enhancement
Description
Complete L2PS (Layer 2 Private Subnet) and DTR (Distributed Transaction Routing) Implementation
Core Features Implemented:
L2PS Network Management: Refactored to singleton pattern with lazy initialization, transaction encryption/decryption, and signature verification using
UnifiedCryptoL2PS Mempool Manager: New dedicated mempool for L2PS transactions with duplicate detection, hash generation, and comprehensive statistics
Validator Hash Service: Periodic L2PS hash generation and relay to validators every 5 seconds with reentrancy protection
DTR Transaction Routing: Non-validator nodes relay transactions to validators with parallel relay (5 validator concurrency limit) and retry mechanism
L2PS Concurrent Sync: Peer discovery and mempool synchronization across L2PS participants with incremental sync and batch duplicate detection
Instant Messaging Enhancement: Offline message storage with DoS protection, blockchain integration, and nonce-based replay prevention using mutex-based thread-safe operations
Validator Consensus: L2PS hash mapping manager for validators with atomic upsert operations and content-blind privacy model
Background Services: DTR relay retry service with validator caching optimization and L2PS hash generation service with graceful shutdown
Database & Entity Changes:
New
L2PSMempoolentity with composite indexing for efficient transaction queryingNew
L2PSHashesentity for validator consensus with UID-to-hash mappingsNew
OfflineMessagesentity for instant messaging with status trackingUpdated
GCRSubnetsTxsentity to useL2PSTransactiontypeDatasource refactored to singleton pattern
Integration Points:
L2PS transaction handler with mempool integration and signature verification
NodeCall endpoints for L2PS participation queries, mempool info, and transaction retrieval
Concurrent L2PS sync during blockchain synchronization
DTR relay handler with validator status verification
Shared state extensions for DTR caching and L2PS participation tracking
Infrastructure:
Added
async-mutexdependency for synchronization primitivesUpdated
@kynesyslabs/demosdkto ^2.2.71Comprehensive documentation and implementation guides for L2PS phases and DTR architecture
Development guidelines and onboarding documentation for future sessions
Diagram Walkthrough
File Walkthrough
21 files
parallelNetworks.ts
L2PS Network Management Refactored to Singleton Patternsrc/libs/l2ps/parallelNetworks.ts
Subnetclass to newParallelNetworkssingleton managing multiple L2PS networks
conditions during
loadL2PS()signature verification using
UnifiedCryptoL2PSNodeConfiginterface for configuration management withfile-based key loading and validation
processL2PSTransaction()formempool integration
signalingServer.ts
Instant Messaging with Offline Storage and Nonce Managementsrc/features/InstantMessagingProtocol/signalingServer/signalingServer.ts
uniqueness and replay prevention
per sender with
MAX_OFFLINE_MESSAGES_PER_SENDER)mandatory audit trail consistency
transactional semantics and WebSocket state checking
atomic operations via
Mutexl2ps_mempool.ts
L2PS Mempool Manager with Hash Generationsrc/libs/blockchain/l2ps_mempool.ts
locking for race condition prevention
deterministic ordering for validator relay
hashes
comprehensive statistics
L2PSHashService.ts
L2PS Hash Generation and Validator Relay Servicesrc/libs/l2ps/L2PSHashService.ts
every 5 seconds
generation cycles
DemosSDK instance for efficiency instead of creating newinstances per cycle
sequential fallback
support
endpointHandlers.ts
DTR Transaction Routing and L2PS Hash Update Handlingsrc/libs/network/endpointHandlers.ts
nodes to relay transactions to validators
using
Promise.allSettled()handleL2PSHashUpdate()handler for processing L2PS hash updatesfrom other nodes
consensus
relayRetryService.ts
DTR Relay Retry Service with Validator Optimizationsrc/libs/network/dtr/relayRetryService.ts
non-validator nodes to validators
number changes)
indefinite hanging
selection
cache eviction
L2PSConcurrentSync.ts
L2PS Concurrent Sync and Peer Discoverysrc/libs/l2ps/L2PSConcurrentSync.ts
across peers
graceful failure handling
redundant transfers
repository access checks
generation for muid collision prevention
l2ps_hashes.ts
L2PS Hash Mapping Manager for Validator Consensussrc/libs/blockchain/l2ps_hashes.ts
consensus
concurrent updates
monitoring
validators
GCRSubnetsTxs.ts
Update GCR Subnet Transactions Type Definitionsrc/model/entities/GCRv2/GCRSubnetsTxs.ts
tx_datacolumn type fromEncryptedTransactiontoL2PSTransactionhandleL2PS.ts
L2PS transaction handler refactoring with mempool integrationsrc/libs/network/routines/transactions/handleL2PS.ts
error handling for nested data structures
original_hashfieldpayload validation
and L2PS UID tracking
manageNodeCall.ts
DTR relay and L2PS mempool synchronization endpointssrc/libs/network/manageNodeCall.ts
RELAY_TXcase) with validatorstatus verification and transaction validation
getL2PSParticipationById,getL2PSMempoolInfo,getL2PSTransactionsoperations and L2PS mempool queries
mempool insertion
Sync.ts
Concurrent L2PS sync integration with blockchain synchronizationsrc/libs/blockchain/routines/Sync.ts
discoverL2PSParticipants()download via
syncL2PSWithPeer()exchangeL2PSParticipation()blockchain sync performance
index.ts
Background service initialization for DTR and L2PSsrc/index.ts
check
and L2PS services
checking
transaction.ts
Transaction class constructor refactoring for flexibilitysrc/libs/blockchain/transaction.ts
Transactionclass constructor to accept optional partialdata for flexible initialization
assignment
from_ed25519_addressmovedto beginning
Object.assign()for cleaner propertysetup
L2PSMempool.ts
L2PS mempool entity with composite indexingsrc/model/entities/L2PSMempool.ts
JSONB support
l2ps_uidwithtimestampandstatusforefficient querying
preservation and entity purpose
original hash tracking
datasource.ts
Datasource refactoring to singleton patternsrc/model/datasource.ts
singleton pattern
initialization
OfflineMessageentity to entities arrayL2PSHashes.ts
L2PS hash storage entity for validator consensussrc/model/entities/L2PSHashes.ts
mappings for validators
l2ps_uidwith supporting columns for hash,transaction count, and timestamps
model
detection
OfflineMessages.ts
Offline messages entity for instant messagingsrc/model/entities/OfflineMessages.ts
recipient and sender keys
precision loss
sharedState.ts
Shared state extensions for DTR and L2PSsrc/utilities/sharedState.ts
validityDataCacheMap for DTR retry mechanism storingValidityData by transaction hash
l2psJoinedUidsarray to track L2PS networks the nodeparticipates in
ValidityDatatype from demosdk for DTR caching supportmempool_v2.ts
Mempool transaction removal for DTR relaysrc/libs/blockchain/mempool_v2.ts
removeTransaction()static method for DTR relay success cleanupvalidator relay
isValidator.ts
Validator status detection utility functionsrc/libs/consensus/v2/routines/isValidator.ts
for next block
getCommonValidatorSeed()andgetShard()logic forvalidator determination
1 files
handleStep.ts
L2PS import path update to deprecated modulesrc/libs/network/routines/transactions/demosWork/handleStep.ts
location
parallelNetworkstoparallelNetworks_deprecated3 files
pay.ts
Code formatting consistency improvementssrc/features/multichain/routines/executors/pay.ts
calls
server_rpc.ts
Minor formatting fix for response objectsrc/libs/network/server_rpc.ts
consistency
validateUint8Array.ts
Code style consistency improvementssrc/utilities/validateUint8Array.ts
19 files
L2PS_DTR_IMPLEMENTATION.md
L2PS and DTR implementation documentationsrc/libs/l2ps/L2PS_DTR_IMPLEMENTATION.md
integration
L2PS_PHASES.md
L2PS implementation phases and action itemsL2PS_PHASES.md
criteria
plan_of_action_for_offline_messages.md
Offline messaging and L2PS quantum-safe encryption plansrc/features/InstantMessagingProtocol/signalingServer/plan_of_action_for_offline_messages.md
README.md
DTR implementation overview and architecture guidedtr_implementation/README.md
architecture
L2PS_TESTING.md
L2PS Testing and Validation Guide CreationL2PS_TESTING.md
implementation
phase-by-phase testing
validation procedures
runtime validation
session_2025_01_31_l2ps_completion.md
Session Summary for L2PS Implementation Completion.serena/memories/session_2025_01_31_l2ps_completion.md
finished (100%)
endpoints, concurrent sync, and blockchain integration
and non-blocking operations
validation
DTR_MINIMAL_IMPLEMENTATION.md
DTR Minimal Implementation Plan and Strategydtr_implementation/DTR_MINIMAL_IMPLEMENTATION.md
strategy leveraging existing infrastructure
endpointHandlers.tswith ~20 lines of DTR logic
with block-aware optimization
production implementation
l2ps_onboarding_guide.md
L2PS Onboarding Guide for Future Sessions.serena/memories/l2ps_onboarding_guide.md
privacy model
layer) and all implementation phases
concepts for future LLM sessions
checklist
l2ps_architecture.md
L2PS Architecture Documentation with Diagrams.serena/memories/l2ps_architecture.md
component interactions
and validator hash updates
characteristics
l2ps_implementation_status.md
L2PS Implementation Status - All Phases Complete.serena/memories/l2ps_implementation_status.md
(NodeCall endpoints), 3c-2 (concurrent sync), and 3c-3 (blockchain
integration)
production code
available
l2ps_remaining_work.md
L2PS Remaining Work Documentation.serena/memories/l2ps_remaining_work.md
completed)
implementation details
and sync service
priorities
l2ps_code_patterns.md
L2PS Code Patterns and Conventions Reference.serena/memories/l2ps_code_patterns.md
implementation
database patterns
ParallelNetworks, and PeerManager
development
development_guidelines.md
Development Guidelines and Best Practices.serena/memories/development_guidelines.md
architecture, and best practices
standards
and use linting for validation
development workflow summary
codebase_structure.md
Codebase Structure and Organization Reference.serena/memories/codebase_structure.md
and feature modules
documentation locations
@/prefix and naming conventions forrepository
suggested_commands.md
Suggested Commands Reference Guide.serena/memories/suggested_commands.md
testing, and node operations
code_style_conventions.md
Code Style and Conventions Reference.serena/memories/code_style_conventions.md
PascalCase for classes)
and trailing commas
@/path aliases insteadof relative paths
task_completion_checklist.md
Task Completion Checklist and Validation Guide.serena/memories/task_completion_checklist.md
integration
bun run lint:fixexecution before marking taskscomplete
considerations
validator_status_minimal.md
Validator Status Minimal Implementation Approachdtr_implementation/validator_status_minimal.md
single function
getShard,getCommonValidatorSeed) with zero modificationsisValidatorForNextBlock()function and optionalgetValidatorsForRelay()helpereffectively
tech_stack.md
Tech Stack and Dependencies Reference.serena/memories/tech_stack.md
dependencies
frameworks
Docker and PostgreSQL
settings
3 files
extensions.json
VSCode extension recommendation addition.vscode/extensions.json
nur-publisher.hypercomments-vscodeextension to recommendedextensions list
project.yml
Serena Project Configuration File.serena/project.yml
package.json
Package Configuration and Dependency Updatespackage.json
lint:fixcommand to excludelocal_tests/**directory fromlinting
@kynesyslabs/demosdkfrom ^2.2.70 to ^2.2.71async-mutex^0.5.0 as new dependency for synchronizationprimitives
4 files
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.