From 2683adda577884373cdec636b3893ffc169c59a2 Mon Sep 17 00:00:00 2001 From: Tim Schwartz Date: Mon, 13 Apr 2026 13:35:20 -0500 Subject: [PATCH 1/4] feat: Update feature directory and add performance & deduplication cleanup specifications --- .specify/feature.json | 2 +- .../checklists/requirements.md | 35 +++++ specs/019-perf-dedup-cleanup/spec.md | 146 ++++++++++++++++++ 3 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 specs/019-perf-dedup-cleanup/checklists/requirements.md create mode 100644 specs/019-perf-dedup-cleanup/spec.md diff --git a/.specify/feature.json b/.specify/feature.json index 7753cab..7227bfe 100644 --- a/.specify/feature.json +++ b/.specify/feature.json @@ -1,3 +1,3 @@ { - "feature_directory": "specs/018-audit-bug-security-fixes" + "feature_directory": "specs/019-perf-dedup-cleanup" } diff --git a/specs/019-perf-dedup-cleanup/checklists/requirements.md b/specs/019-perf-dedup-cleanup/checklists/requirements.md new file mode 100644 index 0000000..035feb5 --- /dev/null +++ b/specs/019-perf-dedup-cleanup/checklists/requirements.md @@ -0,0 +1,35 @@ +# Specification Quality Checklist: Performance & Deduplication Cleanup + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-04-13 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- All items pass after one revision (removed C++ type names and code-level patterns from FRs and Key Entities). +- Spec is ready for `/speckit.clarify` or `/speckit.plan`. diff --git a/specs/019-perf-dedup-cleanup/spec.md b/specs/019-perf-dedup-cleanup/spec.md new file mode 100644 index 0000000..0831417 --- /dev/null +++ b/specs/019-perf-dedup-cleanup/spec.md @@ -0,0 +1,146 @@ +# Feature Specification: Performance & Deduplication Cleanup + +**Feature Branch**: `019-perf-dedup-cleanup` +**Created**: 2026-04-13 +**Status**: Draft +**Input**: User description: "Address Performance Issues and Code Duplication" + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Faster Peer Operations Under Load (Priority: P1) + +As a node operator running a blockchain node with many connected peers, I need peer lookups, additions, removals, and ban checks to remain fast regardless of how many peers are stored, so that the node stays responsive during high-traffic periods such as block propagation storms or peer exchange floods. + +**Why this priority**: Every inbound connection attempt, peer exchange message, and block reception triggers peer lookups. With the current linear scan over up to 256 peers, these operations compound during high-activity windows and directly degrade node responsiveness. This is the highest-traffic hot path in the system. + +**Independent Test**: Can be tested by measuring peer lookup/add/remove/ban-check times with a full peer list (256 entries) before and after the change. Delivers immediate latency improvement for all peer-related operations. + +**Acceptance Scenarios**: + +1. **Given** a node with 256 stored peers, **When** a peer lookup is performed, **Then** the lookup completes in constant time regardless of total peer count. +2. **Given** a node with 256 stored peers and a ban list of 50 entries, **When** `is_banned()` is called, **Then** the check completes in constant time regardless of ban list size. +3. **Given** a full peer list, **When** a new peer is added or an existing peer is removed, **Then** the operation completes in constant time. +4. **Given** the peer data structure has been changed, **When** `get_non_banned_peer_addresses()` is called, **Then** it still returns the correct filtered list of non-banned peer addresses. + +--- + +### User Story 2 - Efficient RPC Request Routing (Priority: P2) + +As a developer or application integrating with the node via JSON-RPC, I need RPC requests to be dispatched efficiently and the handler code to be organized into individually testable units, so that adding new RPC methods does not require modifying a single monolithic function. + +**Why this priority**: The RPC interface is the primary integration surface. The current 21-branch if/else chain in a 700-line callback makes it hard to maintain, test, and extend. Extracting a dispatch table improves both runtime efficiency and developer maintainability. + +**Independent Test**: Can be tested by sending JSON-RPC requests for each method and verifying correct responses. Handler functions can be unit-tested in isolation. + +**Acceptance Scenarios**: + +1. **Given** a running node with RPC enabled, **When** any supported JSON-RPC method is called, **Then** the response is identical to the current behavior. +2. **Given** the RPC dispatch mechanism, **When** a request arrives for a supported method, **Then** the correct handler is invoked without walking a chain of string comparisons. +3. **Given** the refactored RPC handlers, **When** a new RPC method needs to be added, **Then** a developer can register it by adding a single entry to a dispatch table and implementing one handler function. +4. **Given** any unsupported method name, **When** a JSON-RPC request arrives, **Then** a standard JSON-RPC "method not found" error is returned. + +--- + +### User Story 3 - Consolidated Packet Serialization (Priority: P2) + +As a developer maintaining the P2P networking code, I need the shared packet serialization logic between PeerClient and PeerServer to live in one place, so that bug fixes or protocol changes only need to be made once. + +**Why this priority**: The duplicated serialize → PacketHeader → memcpy → async_write pattern in PeerClient and PeerServer is a maintenance risk — a fix applied to one side but not the other could introduce subtle protocol bugs. Consolidating this reduces defect surface area. + +**Independent Test**: Can be tested by running the existing P2P integration tests (peer connection, sync, block propagation) and verifying identical behavior after consolidation. + +**Acceptance Scenarios**: + +1. **Given** PeerClient sends a packet, **When** the shared serialization utility is used, **Then** the wire format is identical to the current format. +2. **Given** PeerServer sends a packet, **When** the shared serialization utility is used, **Then** the wire format is identical to the current format. +3. **Given** a protocol change to the serialization format, **When** a developer updates the shared utility, **Then** both PeerClient and PeerServer reflect the change. + +--- + +### User Story 4 - Consolidated Test Helpers (Priority: P3) + +As a developer writing or maintaining tests, I need all test utility functions (mining helpers, chain builders, temporary directory management) to live in the shared `TestHelpers.hpp` rather than being duplicated across four test files, so that test setup is consistent and easy to maintain. + +**Why this priority**: While lower impact than runtime improvements, duplicated test helpers create drift risk — changes to block creation logic may not be reflected in all local copies, causing false passes or confusing failures. + +**Independent Test**: Can be tested by running the full test suite and verifying all tests pass after replacing local helpers with shared ones. + +**Acceptance Scenarios**: + +1. **Given** the shared `TestHelpers.hpp`, **When** `sync_tests.cpp`, `block_propagation_tests.cpp`, `consensus_tests.cpp`, and `chunk_persistence_tests.cpp` are compiled, **Then** none of them define local versions of `mineTestBlock()`, `buildValidChain()`, or temporary directory helpers. +2. **Given** a change to the shared `mineTestBlock()` function, **When** all test files are recompiled, **Then** every test that uses block mining picks up the updated logic. +3. **Given** the refactored test files, **When** the full test suite is run, **Then** all tests produce the same pass/fail results as before the consolidation. + +--- + +### User Story 5 - Reduced Unnecessary Log Allocations (Priority: P3) + +As a node operator running a production node, I need log calls to avoid constructing expensive string arguments when the current log level would suppress the message, so that logging overhead is minimized in production. + +**Why this priority**: String construction in suppressed log calls is wasteful but low-severity — it affects CPU and memory marginally. This is a clean-up item that improves overall code quality. + +**Independent Test**: Can be tested by verifying that, under a high log level (e.g., ERROR only), DEBUG/INFO log calls do not allocate temporary strings. + +**Acceptance Scenarios**: + +1. **Given** the log level is set to ERROR, **When** a DEBUG-level log call is reached, **Then** no string formatting or allocation occurs. +2. **Given** the log level is set to INFO, **When** an INFO-level log call is reached, **Then** the message is formatted and logged normally. + +--- + +### Edge Cases + +- What happens when a peer is looked up that does not exist in the map? The system must handle missing keys gracefully (return not-found, not crash). +- What happens when the ban list is empty? Ban checks must return false (not banned) without errors. +- What happens when an RPC request arrives with a valid JSON-RPC envelope but an empty method name? The dispatch table must return a "method not found" error. +- What happens when PeerClient and PeerServer serialization paths diverge due to a merge conflict? The shared utility ensures a single source of truth, eliminating this class of error. +- What happens when code assumes a specific peer iteration order? The peer container does not guarantee ordering; callers and tests must not depend on insertion order. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: System MUST perform peer lookup, addition, removal, and ban-check operations in constant time regardless of total peer count. +- **FR-002**: System MUST perform ban-status checks in constant time regardless of the number of ban records. +- **FR-003**: System MUST dispatch RPC requests via a table-based lookup that maps method names to handler functions, replacing the current sequential comparison chain. +- **FR-004**: System MUST return a standard JSON-RPC "method not found" error (-32601) for unsupported method names. +- **FR-010**: Each extracted RPC handler MUST return a complete JSON response (success or error); the dispatcher MUST NOT wrap handlers in a catch-all exception handler. +- **FR-005**: System MUST consolidate the duplicated outbound packet serialization logic (currently in both PeerClient and PeerServer) into a single shared header-only template utility. +- **FR-006**: The shared serialization utility MUST produce wire-compatible output with the current format (no protocol break). +- **FR-007**: System MUST consolidate all duplicated test utility functions (block mining helpers, chain builders, temporary directory management) into the existing shared test helpers module, removing local copies from individual test files. +- **FR-008**: System MUST provide a mechanism to skip message formatting for log calls when the message would be suppressed by the current log level. +- **FR-009**: All existing tests MUST continue to pass after these changes (no behavioral regressions). + +### Key Entities + +- **PeerEntry**: Represents a connected or known peer. Key attributes: host, port, last-seen timestamp, connection state. +- **BanRecord**: Represents a banned peer. Key attributes: peer address, ban expiry time. +- **RPC Handler**: A callable that processes a single JSON-RPC method. Key attributes: method name, handler function. +- **PacketSerializer**: Shared utility for serializing outbound P2P packets. Used by both client and server networking components. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: Peer lookup, add, remove, and ban-check operations complete in constant time regardless of peer count (verified by algorithmic analysis — O(1) vs former O(n)). +- **SC-002**: RPC method dispatch completes in constant time regardless of the number of registered methods (O(1) hash lookup vs former O(n) string comparisons). +- **SC-003**: The packet serialization logic exists in exactly one location in the codebase — zero duplication between PeerClient and PeerServer. +- **SC-004**: Zero local definitions of `mineTestBlock()`, `buildValidChain()`, or temporary directory helpers exist outside `TestHelpers.hpp`. +- **SC-005**: The full test suite passes with identical results before and after all changes. +- **SC-006**: Suppressed log calls produce zero heap allocations for message formatting (verified by code inspection or instrumentation). + +## Assumptions + +- The maximum peer count remains capped at 256 as configured today; the optimization is worthwhile even at this scale due to frequency of lookups. +- The existing P2P wire format is stable and will not change as part of this feature; the shared serialization utility preserves byte-level compatibility. +- `TestHelpers.hpp` already provides suitable signatures for the helpers being consolidated; minor signature adjustments may be needed to cover all four test files' usage patterns. +- The current logging utility (`logMessage()`) can be extended with a level-check macro or wrapper without changing its public API for existing callers. +- No new RPC methods are being added in this feature; the dispatch table refactor covers exactly the 21 methods that exist today. + +## Clarifications + +### Session 2026-04-13 + +- Q: What error-handling strategy should the extracted RPC handlers use? → A: Each handler returns a JSON result (success or error); the dispatcher does not catch exceptions from handlers. Handlers own their error responses. +- Q: Where should the shared packet serialization utility live? → A: Header-only free function template in a dedicated header file, since the function must be a template to accept any serializable type. +- Q: Should the refactored peer container preserve insertion order? → A: No, arbitrary order is acceptable. Tests that depend on peer ordering must be updated. From 076bb694e81291ee3b337586a0339ea19ee08771 Mon Sep 17 00:00:00 2001 From: Tim Schwartz Date: Mon, 13 Apr 2026 13:44:08 -0500 Subject: [PATCH 2/4] feat: Add performance and deduplication cleanup specifications, including RPC dispatch refactor and data model updates --- .github/copilot-instructions.md | 4 +- .../contracts/rpc-dispatch.md | 69 +++++++ specs/019-perf-dedup-cleanup/data-model.md | 80 +++++++++ specs/019-perf-dedup-cleanup/plan.md | 85 +++++++++ specs/019-perf-dedup-cleanup/quickstart.md | 52 ++++++ specs/019-perf-dedup-cleanup/research.md | 168 ++++++++++++++++++ 6 files changed, 457 insertions(+), 1 deletion(-) create mode 100644 specs/019-perf-dedup-cleanup/contracts/rpc-dispatch.md create mode 100644 specs/019-perf-dedup-cleanup/data-model.md create mode 100644 specs/019-perf-dedup-cleanup/plan.md create mode 100644 specs/019-perf-dedup-cleanup/quickstart.md create mode 100644 specs/019-perf-dedup-cleanup/research.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 64291e6..0350550 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -22,6 +22,8 @@ Auto-generated from all feature plans. Last updated: 2026-04-13 - N/A (build-system-only change) (015-compile-time-optimization) - C++20 (`-std=c++20`) + Boost (Asio, Serialization), OpenSSL (EVP SHA-256), nlohmann/json (vendored `src/json.hpp`) (016-audit-remediation) - Boost.Serialization binary chunk files (`chunk_NNNNNN.dat`), keys (`keys.dat`), streams (`streams.dat`, `stream_index.dat`) (016-audit-remediation) +- C++20 (`-std=c++20`) + Boost (Asio, Serialization), OpenSSL (EVP SHA-256), nlohmann/json (vendored `src/json.hpp`), Catch2 (test only) (019-perf-dedup-cleanup) +- Boost.Serialization binary chunk files (`chunk_NNNNNN.dat`), JSON files (`peers.json`, `config.json`) (019-perf-dedup-cleanup) - C++20 (`-std=c++20`) + Boost (Asio, Serialization), OpenSSL, nlohmann/json (vendored `src/json.hpp`), Catch2 (test only) (001-code-constitution-audit) @@ -49,9 +51,9 @@ C++20 (`-std=c++20`): Follow standard conventions - Do not include task numbers (e.g. T001, T010) in code comments. Comments should describe *what* or *why*, not reference planning artifacts. ## Recent Changes +- 019-perf-dedup-cleanup: Added C++20 (`-std=c++20`) + Boost (Asio, Serialization), OpenSSL (EVP SHA-256), nlohmann/json (vendored `src/json.hpp`), Catch2 (test only) - 018-audit-bug-security-fixes: Added C++20 (`-std=c++20`) + Boost (Asio, Serialization), OpenSSL (EVP SHA-256), nlohmann/json (vendored `src/json.hpp`) - 017-blockchain-module-split: Added C++20 (`-std=c++20`) + Boost (Asio, Serialization), OpenSSL (EVP SHA-256), nlohmann/json (vendored `src/json.hpp`) -- 016-audit-remediation: Added C++20 (`-std=c++20`) + Boost (Asio, Serialization), OpenSSL (EVP SHA-256), nlohmann/json (vendored `src/json.hpp`) diff --git a/specs/019-perf-dedup-cleanup/contracts/rpc-dispatch.md b/specs/019-perf-dedup-cleanup/contracts/rpc-dispatch.md new file mode 100644 index 0000000..11005b4 --- /dev/null +++ b/specs/019-perf-dedup-cleanup/contracts/rpc-dispatch.md @@ -0,0 +1,69 @@ +# JSON-RPC Contract: 019-perf-dedup-cleanup + +**Date**: 2026-04-13 + +## Overview + +This feature refactors the RPC dispatch mechanism. The JSON-RPC interface +exposed to external clients does **not change** — all 20 methods retain +identical request/response formats. This contract documents the existing +interface for verification that the refactor is behavior-preserving. + +## Method Registry + +All methods below must be present in the dispatch table after refactoring. +Each method returns a JSON-RPC 2.0 response. + +| Method | Params Required | Success Response | Error Codes | +|--------|----------------|------------------|-------------| +| `publish` | `stream`, `key`; optional: `data`, `keys` | Block JSON | -32602 (invalid params), -32000 (mining timeout), -32001 (syncing), -32003 (stream not permitted) | +| `createStream` | `name` | Stream name | -32602, -32000, -32001 | +| `listStreams` | none | JSON array of streams | — | +| `getStreamEntries` | `stream` | JSON array of entries | -32602 | +| `getStreamEntry` | `stream`, `key` | Entry JSON | -32602 | +| `requestSync` | none | `"sync_started"` | -32002 (already syncing), -32003 (no peer) | +| `getBlockByIndex` | `index` | Block JSON | -32602, -32001 (not found) | +| `getBlocksByKeys` | `keys` | JSON array of blocks | -32602 | +| `addPeer` | `host`, `port` | Success message | -32602 | +| `removePeer` | `host`, `port` | Success message | -32602 | +| `listPeers` | none | JSON array of peers | — | +| `banPeer` | `host`, `port`; optional: `reason`, `duration` | Success message | -32602 | +| `unbanPeer` | `host`, `port` | Success message | -32602 | +| `getInclusionProof` | `blockIndex`, `entryIndex` | Proof JSON | -32602 | +| `verifyInclusionProof` | `proof` | Boolean result | -32602 | +| `getBlockHeader` | `index` | Header JSON | -32602 | +| `getNodeStatus` | none | Status JSON | — | +| `getBlockRange` | `start`, `end` | JSON array of blocks | -32602 | +| `getChainLength` | none | Length string | — | +| `getChunkCount` | none | Count string | — | + +## Error Response Format + +All error responses use the existing `errorMessage()` helper: + +```json +{ + "jsonrpc": "2.0", + "error": { + "code": -32601, + "message": "Invalid method: unknownMethod" + }, + "id": 1 +} +``` + +Standard error codes: +- `-32600`: Invalid JSON-RPC message +- `-32601`: Method not found +- `-32602`: Invalid parameters +- `-32000`: Mining timeout / internal error +- `-32001`: Block not found / sync in progress +- `-32002`: Sync already in progress +- `-32003`: No peer connected / stream not permitted + +## Verification + +After refactoring, every method in this table must produce byte-identical +JSON responses for the same input. The existing `rpc_integration_tests` +exercise the live RPC interface over SSL sockets and serve as the primary +regression gate. diff --git a/specs/019-perf-dedup-cleanup/data-model.md b/specs/019-perf-dedup-cleanup/data-model.md new file mode 100644 index 0000000..1231898 --- /dev/null +++ b/specs/019-perf-dedup-cleanup/data-model.md @@ -0,0 +1,80 @@ +# Data Model: Performance & Deduplication Cleanup + +**Date**: 2026-04-13 +**Feature**: 019-perf-dedup-cleanup + +## Modified Entities + +### PeerEntry (existing — container change only) + +No field changes. The storage container changes from `std::vector` to `std::unordered_map` keyed by `peer_key(host, port)`. + +| Field | Type | Description | +|-------|------|-------------| +| host | string | Peer hostname/IP (normalized) | +| port | uint16_t | Peer listen port | +| node_uuid | string | Remote node UUID | +| last_seen | uint64_t | Unix timestamp of last contact | +| error_count | uint32_t | Consecutive error count | + +**Key**: `peer_key(host, port)` → `"host:port"` string +**Serialization**: JSON (to/from `peers.json`) — unchanged format +**Validation**: Host is normalized via `normalize_address()` before key construction + +### BanRecord (existing — container change only) + +No field changes. The storage container changes from `std::vector` to `std::unordered_map` keyed by `peer_key(host, port)`. + +| Field | Type | Description | +|-------|------|-------------| +| host | string | Banned peer hostname/IP | +| port | uint16_t | Banned peer port | +| reason | string | Ban reason | +| expires | uint64_t | Unix timestamp; 0 = permanent | + +**Key**: `peer_key(host, port)` → `"host:port"` string +**Serialization**: JSON (to/from `peers.json`) — unchanged format +**State transition**: `expires == 0` → permanent ban; `expires > 0 && expires <= now` → expired (removed by `purge_expired_bans()`) + +## New Entities + +### RPC Dispatch Table + +A mapping from JSON-RPC method names to handler functions, stored as a private member of `RpcServer`. + +| Field | Type | Description | +|-------|------|-------------| +| method_name | string (key) | JSON-RPC method name (e.g., "publish") | +| handler | function | Callable returning JSON response | + +**Type**: `std::unordered_map>` +**Lifecycle**: Initialized once in `RpcServer` constructor; immutable thereafter +**Cardinality**: Exactly 20 entries (one per existing RPC method) + +### PacketSerializer (utility — no persistent state) + +A header-only template function that serializes an object with Boost.Serialization and prepends a `PacketHeader`. No stored state — pure function. + +**Input**: Serializable object of type T, packet type enum value +**Output**: Pair of (header bytes, serialized payload string) +**Wire format**: `[PacketHeader: 16 bytes][serialized payload: N bytes]` — identical to current format + +## Relationships + +``` +PeerManager + ├── peers_: unordered_map (was: vector) + ├── bans_: unordered_map (was: vector) + └── peer_key(host, port) → string key (existing static helper) + +RpcServer + └── dispatch_: unordered_map (new) + └── 20 handler methods (extracted from do_read) + +PeerClient::send() ──uses──▶ serialize_packet() (PacketSerializer.hpp) +PeerServer::send_packet() ──uses──▶ serialize_packet() (PacketSerializer.hpp) +``` + +## On-Disk Format + +No changes. `peers.json` continues to store peers as a JSON array and bans as a JSON array. The internal container type is transparent to the serialized format. diff --git a/specs/019-perf-dedup-cleanup/plan.md b/specs/019-perf-dedup-cleanup/plan.md new file mode 100644 index 0000000..266f78c --- /dev/null +++ b/specs/019-perf-dedup-cleanup/plan.md @@ -0,0 +1,85 @@ +# Implementation Plan: Performance & Deduplication Cleanup + +**Branch**: `019-perf-dedup-cleanup` | **Date**: 2026-04-13 | **Spec**: [spec.md](spec.md) +**Input**: Feature specification from `/specs/019-perf-dedup-cleanup/spec.md` + +## Summary + +Resolve four performance issues (O(n) peer lookups, O(n) RPC dispatch, duplicated packet serialization, eager log formatting) and two code duplication clusters (PeerClient/PeerServer send templates, test helpers) identified in the codebase audit. All changes are internal refactors — no protocol changes, no new features, no public API changes. + +## Technical Context + +**Language/Version**: C++20 (`-std=c++20`) +**Primary Dependencies**: Boost (Asio, Serialization), OpenSSL (EVP SHA-256), nlohmann/json (vendored `src/json.hpp`), Catch2 (test only) +**Storage**: Boost.Serialization binary chunk files (`chunk_NNNNNN.dat`), JSON files (`peers.json`, `config.json`) +**Testing**: Catch2 (unit + integration), each test binary run individually +**Target Platform**: Linux, macOS, Windows (cross-platform required) +**Project Type**: CLI / daemon — blockchain node with P2P networking and JSON-RPC interface +**Performance Goals**: O(1) peer lookups and RPC dispatch; zero heap allocation for suppressed log calls +**Constraints**: Wire-format compatibility (no P2P protocol break); all 20 existing RPC methods must return identical responses +**Scale/Scope**: Max 256 stored peers, 20 RPC methods, 4 test files with duplicated helpers + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +| Principle | Status | Notes | +|-----------|--------|-------| +| I. Language Standard (C++20) | PASS | All changes use C++20; no later features | +| II. Build System (Autotools, make -j8) | PASS | New header-only file needs no Makefile.am change; no build system migration | +| III. Full Test Coverage (Catch2, individual binaries) | PASS | Existing tests are refactored, not removed; no new test binaries needed | +| IV. Code Style | PASS | Follow existing conventions (naming, indentation, `#pragma once`) | +| V. Minimal Dependencies | PASS | No new dependencies; uses only approved set | +| VI. Mandatory TLS | N/A | No network protocol changes | +| VII. Cross-Platform Support | PASS | `std::unordered_map`, preprocessor-guarded log macro compatible with all three targets | +| VIII. Feature Branches with PRs | PASS | Working on `019-perf-dedup-cleanup` branch | +| IX. Pre-1.0 API Stability | N/A | No API changes | +| X. Low-Latency Performance | PASS | This feature specifically improves hot-path latency | +| XI. MIT License | PASS | No third-party code added | +| XII. .gitignore Maintenance | N/A | No new build targets | +| XIII. Roadmap Currency | PASS | Will update `docs/ROADMAP.md` on completion | + +**Gate result**: PASS — no violations. + +## Project Structure + +### Documentation (this feature) + +```text +specs/019-perf-dedup-cleanup/ +├── plan.md # This file +├── research.md # Phase 0 output +├── data-model.md # Phase 1 output +├── quickstart.md # Phase 1 output +├── contracts/ # Phase 1 output (RPC contract) +└── tasks.md # Phase 2 output (speckit.tasks) +``` + +### Source Code (repository root) + +```text +src/ +├── PeerManager.hpp # Modified: vector→unordered_map for peers_; vector→unordered_map for bans_ +├── PeerManager.cpp # Modified: all peer/ban operations updated for map API +├── utils.hpp # Modified: add LOG_MSG macro +├── network/ +│ ├── PacketHeader.hpp # Unchanged +│ ├── PacketSerializer.hpp # NEW: header-only shared send template +│ ├── PeerClient.cpp # Modified: replace send body with PacketSerializer call +│ ├── PeerServer.cpp # Modified: replace send_packet body with PacketSerializer call +│ ├── RpcServer.hpp # Modified: add dispatch table type + registration +│ └── RpcServer.cpp # Modified: extract handlers, replace if/else with dispatch table + +tests/ +├── TestHelpers.hpp # Modified: add make_block() variant; ensure all helper signatures cover all usages +├── sync_tests.cpp # Modified: remove local mineTestBlock/buildValidChain, use TestHelpers +├── consensus_tests.cpp # Modified: remove local mineBlock, use TestHelpers +├── block_propagation_tests.cpp # Modified: remove local helpers, use TestHelpers +└── chunk_persistence_tests.cpp # Modified: remove local make_block, use TestHelpers +``` + +**Structure Decision**: No new directories or binaries. One new header-only file (`PacketSerializer.hpp`) in existing `src/network/`. All other changes are modifications to existing files. + +## Complexity Tracking + +No constitution violations — this section is empty. diff --git a/specs/019-perf-dedup-cleanup/quickstart.md b/specs/019-perf-dedup-cleanup/quickstart.md new file mode 100644 index 0000000..a04fbaa --- /dev/null +++ b/specs/019-perf-dedup-cleanup/quickstart.md @@ -0,0 +1,52 @@ +# Quickstart: Performance & Deduplication Cleanup + +## What This Feature Changes + +Five internal refactors — no user-facing protocol or API changes: + +1. **Peer lookups** become O(1) instead of O(n) +2. **RPC dispatch** becomes O(1) instead of O(n) and handlers are individually testable +3. **Packet serialization** is deduplicated between PeerClient and PeerServer +4. **Test helpers** are consolidated into TestHelpers.hpp +5. **Log formatting** is skipped for suppressed messages + +## Build & Test + +```bash +# Build (constitution requires -j8) +make -j8 + +# Run test binaries individually (constitution requirement) +./tests/blockchain_tests +./tests/block_propagation_tests +./tests/chunk_persistence_tests +./tests/lifecycle_tests +./tests/rpc_integration_tests +./tests/p2p_sync_integration_tests +``` + +## Key Files to Review + +| File | Change Type | What Changed | +|------|------------|--------------| +| `src/PeerManager.hpp` | Modified | `peers_` and `bans_` container types | +| `src/PeerManager.cpp` | Modified | All peer/ban CRUD operations | +| `src/network/RpcServer.hpp` | Modified | Dispatch table type declaration | +| `src/network/RpcServer.cpp` | Modified | Handlers extracted, dispatch table | +| `src/network/PacketSerializer.hpp` | **New** | Shared serialization template | +| `src/network/PeerClient.cpp` | Modified | Uses PacketSerializer | +| `src/network/PeerServer.cpp` | Modified | Uses PacketSerializer | +| `src/utils.hpp` | Modified | LOG_MSG / LOG_INFO / etc. macros | +| `tests/TestHelpers.hpp` | Modified | Added `make_block()` helper | +| `tests/sync_tests.cpp` | Modified | Removed local helpers | +| `tests/consensus_tests.cpp` | Modified | Removed local helpers | +| `tests/block_propagation_tests.cpp` | Modified | Removed local helpers | +| `tests/chunk_persistence_tests.cpp` | Modified | Removed local helpers | + +## Verification Checklist + +- [ ] All 20 RPC methods return identical responses (run `rpc_integration_tests`) +- [ ] P2P sync and block propagation work (run `p2p_sync_integration_tests`) +- [ ] No local `mineTestBlock` / `buildValidChain` / `make_block` definitions outside TestHelpers.hpp +- [ ] `peers.json` loads correctly from existing files (no migration needed) +- [ ] `LOG_INFO(...)` macro compiles and works on Linux, macOS, Windows diff --git a/specs/019-perf-dedup-cleanup/research.md b/specs/019-perf-dedup-cleanup/research.md new file mode 100644 index 0000000..07f9dbf --- /dev/null +++ b/specs/019-perf-dedup-cleanup/research.md @@ -0,0 +1,168 @@ +# Research: Performance & Deduplication Cleanup + +**Date**: 2026-04-13 +**Feature**: 019-perf-dedup-cleanup + +## R1: Peer Container Migration (vector → unordered_map) + +### Decision +Replace `std::vector peers_` with `std::unordered_map` keyed by `peer_key(host, port)`. Replace `std::vector bans_` with `std::unordered_map` keyed by the same key format. + +### Rationale +- `peer_key()` already exists as a `static` helper in PeerManager — it constructs `"host:port"` strings. The infrastructure for consistent key generation is already in place. +- `find_peer()`, `add_peer()`, `remove_peer()`, `is_banned()` all do linear scans with the same key predicate (`host == x && port == y`). Switching to map keyed by `peer_key()` makes all these O(1). +- `get_peers()` returns `std::vector` — this public API can remain unchanged by iterating the map values. +- `get_bans()` returns `std::vector` — same approach. + +### Alternatives Considered +1. **`std::map`**: O(log n) instead of O(1). No advantage at n <= 256 since hash is faster for string keys. +2. **Custom hash on PeerEntry**: Over-engineered; string key from existing `peer_key()` is simple and works. +3. **Keep vector + add index**: Dual data structure maintenance with no benefit. + +### Migration Impact + +| Method | Current Pattern | New Pattern | +|--------|----------------|-------------| +| `find_peer()` | Linear scan | `peers_.find(key)` → iterator | +| `add_peer()` | Linear scan + push_back | `peers_[key]` update or insert | +| `remove_peer()` | `std::remove_if` + erase | `peers_.erase(key)` | +| `is_banned()` | Linear scan of bans_ | `bans_.find(key)` + expiry check | +| `evict_oldest_peer()` | `std::min_element` | `std::min_element` over map values (unchanged algorithmic pattern) | +| `get_peers()` | Return copy of vector | Iterate map values into vector | +| `get_non_banned_peer_addresses()` | Iterate vector | Iterate map values | +| `save_peers()` | `j["peers"] = peers_` | Serialize map values as JSON array | +| `load_peers()` | `peers_ = j["peers"].get()` | Deserialize JSON array into map entries | + +### JSON Backward Compatibility +`peers.json` stores peers as a JSON array. The on-disk format does not change — `save_peers()` will serialize map values as a JSON array, and `load_peers()` will read the array and insert each entry into the map. Existing `peers.json` files load without migration. + +### Pointer Stability +`find_peer()` currently returns `PeerEntry*` — a pointer into the vector. With `std::unordered_map`, pointers/references to values remain stable across insertions (unlike vector). This is an improvement, not a regression. However, `erase()` invalidates the erased element's pointer, which is the expected behavior. + +--- + +## R2: RPC Dispatch Table + +### Decision +Replace the 20-branch if/else chain in `do_read()` with an `std::unordered_map>` dispatch table initialized once on construction. Each handler is a private member function that receives the parsed JSON request and returns a JSON response. + +### Rationale +- 20 methods × string comparison per request = O(n) dispatch. Hash table gives O(1). +- The current `do_read()` is ~700 lines. Extracting handlers into separate functions makes each independently readable and testable. +- The fallback `invalidMethodMessage()` already exists and returns -32601. + +### Handler Signature +``` +using RpcHandler = std::function; +std::unordered_map dispatch_; +``` + +Each handler receives the full JSON-RPC request object and returns a complete JSON response (success or error). Per clarification, the dispatcher does not catch exceptions — handlers own their error responses. + +### Alternatives Considered +1. **`std::map`**: O(log n) — no advantage over hash map for string keys. +2. **Compile-time dispatch (constexpr map)**: Not available in C++20 for runtime strings in a standard way. Over-engineered. +3. **Switch on hash of method name**: Fragile, hash collisions, harder to maintain. + +### Dispatch Loop Pattern +``` +auto it = dispatch_.find(method); +if (it != dispatch_.end()) { + response = it->second(object); +} else { + response = invalidMethodMessage(object["id"], method); +} +``` + +### Static Helpers +The existing static helper functions (`errorMessage`, `resultMessage`, `resultJsonMessage`, etc.) remain static — handlers call them to construct responses. No change to helper signatures. + +--- + +## R3: Shared Packet Serialization + +### Decision +Create `src/network/PacketSerializer.hpp` — a header-only free function template that encapsulates the serialize → PacketHeader → async_write pattern. + +### Rationale +- `PeerClient::send()` and `PeerServer::send_packet()` are nearly identical templates with minor differences in buffer management (PeerClient uses `write_buffer` member; PeerServer uses shared_ptr buffers for lifetime). +- A header-only template avoids needing explicit instantiations in a separate .cpp file. +- The two callers have different socket types (`ssl::stream` vs `boost::asio::streambuf`-based) and different lifetime patterns (PeerServer captures `shared_from_this()`). The shared utility must be parameterized on the write target. + +### Design +The shared function accepts a pre-serialized buffer (string) + packet type + a write callback, leaving socket/lifetime management to each caller. This is the minimal shared surface: + +```cpp +template +std::pair, std::string> serialize_packet(const T &obj, uint64_t packet_type); +``` + +Returns the header bytes and serialized payload. Each caller handles `async_write` with its own socket and lifetime management, since PeerClient and PeerServer have fundamentally different ownership models. + +### Alternatives Considered +1. **Full async_write in shared function**: Would require abstracting over socket types and lifetime (shared_ptr capture). Too much coupling. +2. **Base class with virtual send**: Runtime overhead for a template function. Not idiomatic C++. +3. **Add to PacketHeader.hpp**: PacketHeader.hpp is a POD struct header — adding Boost.Serialization includes would pollute it. + +--- + +## R4: Lazy Log Formatting + +### Decision +Add a preprocessor macro `LOG_MSG(level, expr)` that checks the current log level before evaluating the message expression. The existing `logMessage()` function remains unchanged for backward compatibility. + +### Rationale +- `logMessage()` takes `const std::string &msg` — the string is constructed at the call site before the function can check the level. +- A macro can short-circuit: `if (getLogLevel() <= LogLevel::X) logMessage("X", expr)`. +- The `getLogLevel()` function and `LogLevel` enum already exist. + +### Design +```cpp +#define LOG_MSG(level_str, level_enum, msg_expr) \ + do { if (static_cast(level_enum) >= static_cast(getLogLevel())) \ + logMessage(level_str, msg_expr); } while(0) + +#define LOG_DEBUG(msg) LOG_MSG("DEBUG", LogLevel::Debug, msg) +#define LOG_INFO(msg) LOG_MSG("INFO", LogLevel::Info, msg) +#define LOG_WARN(msg) LOG_MSG("WARN", LogLevel::Warning, msg) +#define LOG_ERROR(msg) LOG_MSG("ERROR", LogLevel::Error, msg) +``` + +### Alternatives Considered +1. **Template with lambda**: `logLazy(LogLevel::Info, [&]{ return "msg " + x; })` — cleaner but requires changing every call site to lambda syntax. Higher migration cost. +2. **Variadic template with fmt-style**: Would require adding fmt or a format library. Violates constitution principle V (minimal dependencies). +3. **Do nothing**: Acceptable at current scale but inconsistent with constitution principle X (low-latency performance). + +### Migration Strategy +New code uses `LOG_INFO(...)` macros. Existing `logMessage()` calls are migrated incrementally — not all in this feature, only the hot-path calls identified in the audit (block propagation, peer exchange, sync). The macro and function coexist indefinitely. + +--- + +## R5: Test Helper Consolidation + +### Decision +Extend `TestHelpers.hpp` with two additional helpers, then remove local definitions from the four test files. + +### Rationale +Comparison of local helpers vs TestHelpers.hpp: + +| Local Helper | File | Key Difference from TestHelpers | +|-------------|------|--------------------------------| +| `mineTestBlock()` | sync_tests.cpp | No merkle root; `difficulty=1` default | +| `mineBlock()` | consensus_tests.cpp | Same as sync_tests but different name | +| `make_block()` | chunk_persistence_tests.cpp | Hardcoded `difficulty=0`, no PoW loop, uses `std::time()` for timestamp | +| `buildValidChain()` | sync_tests.cpp | `difficulty=1` default | + +### New Helpers to Add +1. `make_block(index, prevHash)` — fast block creation with `difficulty=0` and no PoW mining, matching `chunk_persistence_tests.cpp` usage. Uses `static_cast(std::time(nullptr))` for timestamp. +2. No other new helpers needed — the existing `mineTestBlock()` covers `sync_tests.cpp` and `consensus_tests.cpp` usage, and `buildValidChain()` already accepts a difficulty parameter. + +### Key Differences to Resolve +- **Merkle root**: TestHelpers `mineTestBlock()` computes merkle root; local versions do not. The test assertions don't check merkle roots (they test sync/consensus behavior). The shared version with merkle root is strictly more correct — tests pass with it. +- **Default difficulty**: sync_tests uses `difficulty=1`; TestHelpers `buildValidChain` uses `difficulty=0`. The sync_tests callers explicitly pass difficulty, so the default doesn't matter. +- **Timestamp**: chunk_persistence uses `std::time(nullptr)`; others use `index * 10`. The `make_block()` helper preserves the `time(nullptr)` pattern for chunk tests. + +### Migration Plan +1. Add `make_block(index, prevHash)` to `TestHelpers` namespace. +2. In each test file: remove local helper definitions, add `#include "TestHelpers.hpp"`, replace calls (`mineBlock(...)` → `TestHelpers::mineTestBlock(...)`, `make_block(...)` → `TestHelpers::make_block(...)`). +3. Verify all tests pass. From dd0be3dd35eddab7da454d20990ec247219e0f22 Mon Sep 17 00:00:00 2001 From: Tim Schwartz Date: Mon, 13 Apr 2026 13:57:37 -0500 Subject: [PATCH 3/4] feat: Implement performance and deduplication cleanup tasks, including refactoring of peer operations and RPC dispatch --- specs/019-perf-dedup-cleanup/plan.md | 5 + specs/019-perf-dedup-cleanup/spec.md | 8 +- specs/019-perf-dedup-cleanup/tasks.md | 244 ++++++++++++++++++++++++++ 3 files changed, 253 insertions(+), 4 deletions(-) create mode 100644 specs/019-perf-dedup-cleanup/tasks.md diff --git a/specs/019-perf-dedup-cleanup/plan.md b/specs/019-perf-dedup-cleanup/plan.md index 266f78c..1dea40a 100644 --- a/specs/019-perf-dedup-cleanup/plan.md +++ b/specs/019-perf-dedup-cleanup/plan.md @@ -61,6 +61,7 @@ specs/019-perf-dedup-cleanup/ src/ ├── PeerManager.hpp # Modified: vector→unordered_map for peers_; vector→unordered_map for bans_ ├── PeerManager.cpp # Modified: all peer/ban operations updated for map API +├── BlockPropagation.cpp # Modified: logMessage() calls → LOG_* macros ├── utils.hpp # Modified: add LOG_MSG macro ├── network/ │ ├── PacketHeader.hpp # Unchanged @@ -76,6 +77,10 @@ tests/ ├── consensus_tests.cpp # Modified: remove local mineBlock, use TestHelpers ├── block_propagation_tests.cpp # Modified: remove local helpers, use TestHelpers └── chunk_persistence_tests.cpp # Modified: remove local make_block, use TestHelpers + +docs/ +├── AUDIT.md # Modified: mark resolved audit items +└── ROADMAP.md # Modified: move feature to Completed (Constitution §XIII) ``` **Structure Decision**: No new directories or binaries. One new header-only file (`PacketSerializer.hpp`) in existing `src/network/`. All other changes are modifications to existing files. diff --git a/specs/019-perf-dedup-cleanup/spec.md b/specs/019-perf-dedup-cleanup/spec.md index 0831417..b075a3d 100644 --- a/specs/019-perf-dedup-cleanup/spec.md +++ b/specs/019-perf-dedup-cleanup/spec.md @@ -28,7 +28,7 @@ As a node operator running a blockchain node with many connected peers, I need p As a developer or application integrating with the node via JSON-RPC, I need RPC requests to be dispatched efficiently and the handler code to be organized into individually testable units, so that adding new RPC methods does not require modifying a single monolithic function. -**Why this priority**: The RPC interface is the primary integration surface. The current 21-branch if/else chain in a 700-line callback makes it hard to maintain, test, and extend. Extracting a dispatch table improves both runtime efficiency and developer maintainability. +**Why this priority**: The RPC interface is the primary integration surface. The current 20-method if/else chain in a 700-line callback makes it hard to maintain, test, and extend. Extracting a dispatch table improves both runtime efficiency and developer maintainability. **Independent Test**: Can be tested by sending JSON-RPC requests for each method and verifying correct responses. Handler functions can be unit-tested in isolation. @@ -100,8 +100,8 @@ As a node operator running a production node, I need log calls to avoid construc ### Functional Requirements -- **FR-001**: System MUST perform peer lookup, addition, removal, and ban-check operations in constant time regardless of total peer count. -- **FR-002**: System MUST perform ban-status checks in constant time regardless of the number of ban records. +- **FR-001**: System MUST perform peer lookup, addition, and removal operations in amortized constant time regardless of total peer count (cap-triggered eviction may scan all entries). +- **FR-002**: System MUST store ban records in a dedicated constant-time container, separate from peers, so that ban-status checks complete in constant time regardless of the number of ban records. - **FR-003**: System MUST dispatch RPC requests via a table-based lookup that maps method names to handler functions, replacing the current sequential comparison chain. - **FR-004**: System MUST return a standard JSON-RPC "method not found" error (-32601) for unsupported method names. - **FR-010**: Each extracted RPC handler MUST return a complete JSON response (success or error); the dispatcher MUST NOT wrap handlers in a catch-all exception handler. @@ -135,7 +135,7 @@ As a node operator running a production node, I need log calls to avoid construc - The existing P2P wire format is stable and will not change as part of this feature; the shared serialization utility preserves byte-level compatibility. - `TestHelpers.hpp` already provides suitable signatures for the helpers being consolidated; minor signature adjustments may be needed to cover all four test files' usage patterns. - The current logging utility (`logMessage()`) can be extended with a level-check macro or wrapper without changing its public API for existing callers. -- No new RPC methods are being added in this feature; the dispatch table refactor covers exactly the 21 methods that exist today. +- No new RPC methods are being added in this feature; the dispatch table refactor covers exactly the 20 methods that exist today. ## Clarifications diff --git a/specs/019-perf-dedup-cleanup/tasks.md b/specs/019-perf-dedup-cleanup/tasks.md new file mode 100644 index 0000000..1c97640 --- /dev/null +++ b/specs/019-perf-dedup-cleanup/tasks.md @@ -0,0 +1,244 @@ +# Tasks: Performance & Deduplication Cleanup + +**Input**: Design documents from `/specs/019-perf-dedup-cleanup/` +**Prerequisites**: plan.md (required), spec.md (required for user stories), research.md, data-model.md, contracts/ + +**Tests**: Not explicitly requested in the feature specification. Test tasks are omitted. Existing tests are refactored (not new tests). + +**Organization**: Tasks are grouped by user story to enable independent implementation and testing of each story. + +## Format: `[ID] [P?] [Story] Description` + +- **[P]**: Can run in parallel (different files, no dependencies) +- **[Story]**: Which user story this task belongs to (e.g., US1, US2, US3) +- Include exact file paths in descriptions + +--- + +## Phase 1: Setup + +**Purpose**: No project initialization needed — this feature modifies an existing codebase. Phase 1 is a no-op. + +_(No tasks)_ + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**Purpose**: Shared utilities that multiple user stories depend on. Must complete before story work begins. + +- [ ] T001 Add `LOG_DEBUG`, `LOG_INFO`, `LOG_WARN`, `LOG_ERROR` macros to `src/utils.hpp` that check `getLogLevel()` before evaluating the message expression +- [ ] T002 [P] Create `src/network/PacketSerializer.hpp` with header-only `serialize_packet()` template that returns `std::pair, std::string>` (header bytes + serialized payload) +- [ ] T003 [P] Add `make_block(size_t index, const std::string &prevHash)` helper to `tests/TestHelpers.hpp` for fast difficulty-0 block creation matching chunk_persistence_tests usage + +**Checkpoint**: Foundation ready — shared utilities in place for all user stories. + +--- + +## Phase 3: User Story 1 — Faster Peer Operations Under Load (Priority: P1) 🎯 MVP + +**Goal**: Replace O(n) linear scans in PeerManager with O(1) hash map lookups for peers and bans. + +**Independent Test**: Build and run `./tests/blockchain_tests` (contains peer_manager_tests, peer_discovery_tests) — all peer operations exercise the new data structure. + +### Implementation for User Story 1 + +- [ ] T004 [US1] Change `peers_` from `std::vector` to `std::unordered_map` in `src/PeerManager.hpp` and update includes +- [ ] T005 [US1] Change `bans_` from `std::vector` to `std::unordered_map` in `src/PeerManager.hpp` +- [ ] T006 [US1] Rewrite `find_peer()` (both const and non-const overloads) in `src/PeerManager.cpp` to use `peers_.find(peer_key(host, port))` +- [ ] T007 [US1] Rewrite `add_peer()` in `src/PeerManager.cpp` to use map insert-or-update via `peers_[key]`; preserve `normalize_address()` call and cap enforcement with `evict_oldest_peer()` +- [ ] T008 [US1] Rewrite `remove_peer()` in `src/PeerManager.cpp` to use `peers_.erase(peer_key(host, port))` +- [ ] T009 [US1] Rewrite `is_banned()` in `src/PeerManager.cpp` to use `bans_.find(peer_key(host, port))` with expiry check +- [ ] T010 [US1] Update `ban_peer()` and `unban_peer()` in `src/PeerManager.cpp` to use map insert/erase on `bans_` +- [ ] T011 [US1] Update `purge_expired_bans()` in `src/PeerManager.cpp` to iterate and erase expired entries from `bans_` map +- [ ] T012 [US1] Update `get_peers()` in `src/PeerManager.cpp` to collect map values into a `std::vector` +- [ ] T013 [US1] Update `get_bans()` in `src/PeerManager.cpp` to collect map values into a `std::vector` +- [ ] T014 [US1] Update `get_non_banned_peer_addresses()` in `src/PeerManager.cpp` to iterate map values +- [ ] T015 [US1] Update `evict_oldest_peer()` in `src/PeerManager.cpp` to find min `last_seen` across map values and erase by key +- [ ] T016 [US1] Update `save_peers()` in `src/PeerManager.cpp` to serialize map values as a JSON array (preserving `peers.json` format) +- [ ] T017 [US1] Update `load_peers()` in `src/PeerManager.cpp` to deserialize JSON array into map entries using `peer_key()` as key +- [ ] T018 [US1] Update remaining iteration sites in `src/PeerManager.cpp`: `start()`, `on_peer_exchange_received()`, `send_to_peers()`, `increment_error()` to use map iteration +- [ ] T019 [US1] Build with `make -j8` and run `./tests/blockchain_tests` — verify all peer_manager_tests and peer_discovery_tests pass +- [ ] T020 [US1] Run `./tests/p2p_sync_integration_tests` to verify P2P integration with new peer data structure + +**Checkpoint**: Peer operations are O(1). All peer-related tests pass. MVP delivered. + +--- + +## Phase 4: User Story 2 — Efficient RPC Request Routing (Priority: P2) + +**Goal**: Replace the 20-branch if/else chain in `RpcServer::do_read()` with an `std::unordered_map` dispatch table of individually testable handler functions. + +**Independent Test**: Build and run `./tests/rpc_integration_tests` — exercises all 20 RPC methods over live SSL sockets. + +### Implementation for User Story 2 + +- [ ] T021 [US2] Add `RpcHandler` type alias and `dispatch_` member to `src/network/RpcServer.hpp`: `using RpcHandler = std::function;` and `std::unordered_map dispatch_;` +- [ ] T022 [US2] Extract `handle_publish()` as a private method in `src/network/RpcServer.hpp` / `src/network/RpcServer.cpp` — move the `publish` handler body from `do_read()` into this method, returning JSON response +- [ ] T023 [US2] Extract `handle_createStream()` and `handle_listStreams()` as private methods in `src/network/RpcServer.cpp` +- [ ] T024 [P] [US2] Extract `handle_getStreamEntries()` and `handle_getStreamEntry()` as private methods in `src/network/RpcServer.cpp` +- [ ] T025 [P] [US2] Extract `handle_requestSync()` as a private method in `src/network/RpcServer.cpp` +- [ ] T026 [P] [US2] Extract `handle_getBlockByIndex()`, `handle_getBlocksByKeys()`, `handle_getBlockRange()`, `handle_getBlockHeader()` as private methods in `src/network/RpcServer.cpp` +- [ ] T027 [P] [US2] Extract `handle_addPeer()`, `handle_removePeer()`, `handle_listPeers()` as private methods in `src/network/RpcServer.cpp` +- [ ] T028 [P] [US2] Extract `handle_banPeer()`, `handle_unbanPeer()` as private methods in `src/network/RpcServer.cpp` +- [ ] T029 [P] [US2] Extract `handle_getInclusionProof()`, `handle_verifyInclusionProof()` as private methods in `src/network/RpcServer.cpp` +- [ ] T030 [P] [US2] Extract `handle_getNodeStatus()`, `handle_getChainLength()`, `handle_getChunkCount()` as private methods in `src/network/RpcServer.cpp` +- [ ] T031 [US2] Initialize `dispatch_` table in `RpcServer` constructor in `src/network/RpcServer.cpp` — register all 20 handlers by method name string +- [ ] T032 [US2] Replace the if/else chain in `do_read()` in `src/network/RpcServer.cpp` with dispatch table lookup: `auto it = dispatch_.find(method); if (it != dispatch_.end()) response = it->second(object); else response = invalidMethodMessage(...)` — wire `outputStream << response` and `do_write()` after dispatch +- [ ] T033 [US2] Build with `make -j8` and run `./tests/rpc_integration_tests` — verify all 20 RPC methods return identical responses +- [ ] T034 [US2] Run `./tests/rpc_expansion_tests` to verify RPC error code handling is preserved + +**Checkpoint**: RPC dispatch is O(1). Handlers are individually testable. All RPC tests pass. + +--- + +## Phase 5: User Story 3 — Consolidated Packet Serialization (Priority: P2) + +**Goal**: Replace duplicated send templates in PeerClient and PeerServer with the shared `serialize_packet()` utility from Phase 2. + +**Independent Test**: Build and run `./tests/p2p_sync_integration_tests` and `./tests/block_propagation_integration_tests` — exercises P2P packet send/receive paths. + +### Implementation for User Story 3 + +- [ ] T035 [US3] Replace body of `PeerClient::send()` in `src/network/PeerClient.cpp` with call to `serialize_packet()` from `PacketSerializer.hpp`, keeping the existing `async_write` with `write_buffer` and callback +- [ ] T036 [US3] Replace body of `PeerServer::send_packet()` in `src/network/PeerServer.cpp` with call to `serialize_packet()` from `PacketSerializer.hpp`, keeping the existing `async_write` with shared_ptr buffers and callback +- [ ] T037 [US3] Build with `make -j8` and run `./tests/p2p_sync_integration_tests` and `./tests/block_propagation_integration_tests` — verify wire-format compatibility +- [ ] T037a [US3] Add a Catch2 unit test in `tests/blockchain_tests` that calls `serialize_packet()` and verifies the returned header has the correct packet type and payload length matching a manual Boost.Serialization of the same object + +**Checkpoint**: Packet serialization lives in exactly one place. P2P integration tests pass. + +--- + +## Phase 6: User Story 4 — Consolidated Test Helpers (Priority: P3) + +**Goal**: Remove all local test helper definitions from the four test files and use the shared `TestHelpers.hpp` instead. + +**Independent Test**: Build and run the full test suite — all test binaries produce identical pass/fail results. + +### Implementation for User Story 4 + +- [ ] T038 [P] [US4] In `tests/sync_tests.cpp`: remove local `mineTestBlock()` and `buildValidChain()` definitions, add `#include "TestHelpers.hpp"`, replace calls with `TestHelpers::mineTestBlock(...)` and `TestHelpers::buildValidChain(...)` +- [ ] T039 [P] [US4] In `tests/consensus_tests.cpp`: remove local `mineBlock()` definition, add `#include "TestHelpers.hpp"`, replace calls with `TestHelpers::mineTestBlock(...)` +- [ ] T040 [P] [US4] In `tests/block_propagation_tests.cpp`: remove local helper definitions, add `#include "TestHelpers.hpp"`, replace calls with `TestHelpers::` equivalents +- [ ] T041 [P] [US4] In `tests/chunk_persistence_tests.cpp`: remove local `make_block()` definition, add `#include "TestHelpers.hpp"`, replace calls with `TestHelpers::make_block(...)` +- [ ] T042 [US4] Build with `make -j8` and run all test binaries individually — verify identical pass/fail results + +**Checkpoint**: Zero local test helper definitions outside `TestHelpers.hpp`. Full suite passes. + +--- + +## Phase 7: User Story 5 — Reduced Unnecessary Log Allocations (Priority: P3) + +**Goal**: Migrate hot-path `logMessage()` calls to the lazy `LOG_*` macros so suppressed messages produce zero heap allocations. + +**Independent Test**: Code inspection — verify hot-path calls use macros; build and run `./tests/blockchain_tests` to confirm no regressions. + +### Implementation for User Story 5 + +- [ ] T043 [US5] Convert `logMessage()` calls in `src/PeerManager.cpp` hot paths (peer exchange, block relay, connection attempts) to `LOG_INFO(...)` / `LOG_DEBUG(...)` / `LOG_WARN(...)` / `LOG_ERROR(...)` macros +- [ ] T044 [P] [US5] Convert `logMessage()` calls in `src/BlockPropagation.cpp` to `LOG_*` macros +- [ ] T045 [P] [US5] Convert `logMessage()` calls in `src/network/PeerClient.cpp` and `src/network/PeerServer.cpp` to `LOG_*` macros +- [ ] T046 [US5] Build with `make -j8` and run `./tests/blockchain_tests`, `./tests/p2p_sync_integration_tests`, `./tests/block_propagation_integration_tests` — verify no regressions + +**Checkpoint**: Hot-path log calls use lazy macros. Suppressed messages produce zero allocations. + +--- + +## Phase 8: Polish & Cross-Cutting Concerns + +**Purpose**: Documentation updates, audit tracking, and validation. + +- [ ] T047 [P] Update `docs/AUDIT.md` — mark §4.1 (O(n) peer lookups), §4.2 (RPC dispatch), §4.4 (log allocations), §5.1 (packet serialization), and §5.2 (test helpers) as ✅ RESOLVED with references to this feature (019) +- [ ] T048 [P] Update `docs/ROADMAP.md` — move 019-perf-dedup-cleanup from "Suggested Specs" or "In Progress" to "Completed" with one-line summary +- [ ] T049 Run quickstart.md verification checklist: confirm all 5 items pass +- [ ] T050 Final full build (`make -j8`) and run all test binaries individually to confirm zero regressions + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **Setup (Phase 1)**: No-op +- **Foundational (Phase 2)**: No dependencies — can start immediately + - T001 (log macros), T002 (PacketSerializer), T003 (TestHelpers make_block) are independent [P] +- **US1 — Peer Operations (Phase 3)**: No dependency on Phase 2 utilities (doesn't use PacketSerializer or make_block) + - **Can start in parallel with Phase 2** +- **US2 — RPC Dispatch (Phase 4)**: No dependency on Phase 2 or Phase 3 + - **Can start in parallel with Phases 2 and 3** +- **US3 — Packet Serialization (Phase 5)**: Depends on T002 (PacketSerializer.hpp from Phase 2) +- **US4 — Test Helpers (Phase 6)**: Depends on T003 (make_block from Phase 2), and should run after US1 (Phase 3) since peer_manager_tests may be affected +- **US5 — Log Macros (Phase 7)**: Depends on T001 (macros from Phase 2), and should run after US1 (Phase 3) since PeerManager calls are migrated +- **Polish (Phase 8)**: Depends on all user stories being complete + +### User Story Dependencies + +- **US1 (P1)**: Independent — can start immediately +- **US2 (P2)**: Independent — can start immediately +- **US3 (P2)**: Depends on T002 only +- **US4 (P3)**: Depends on T003; best after US1 completes +- **US5 (P3)**: Depends on T001; best after US1 and US3 complete (modified files settle) + +### Within Each User Story + +- Modify header declaration before implementation (.hpp before .cpp changes) +- Core operations before auxiliary operations +- Functional changes before verification (build + test at end of each phase) + +### Parallel Opportunities + +- **Phase 2**: All three foundational tasks (T001, T002, T003) can run in parallel +- **Phase 3 + Phase 4**: US1 and US2 are fully independent — can run in parallel +- **Phase 4 handlers**: T024–T030 extract different handler groups — all can run in parallel +- **Phase 6**: T038–T041 modify different test files — all can run in parallel +- **Phase 7**: T043–T045 modify different source files — T044 and T045 can run in parallel +- **Phase 8**: T047 and T048 modify different doc files — can run in parallel + +--- + +## Parallel Example: Phases 2 + 3 + 4 + +``` +# These three workstreams can proceed simultaneously: + +# Workstream A (Phase 2): Foundational utilities +T001: Add LOG_* macros to src/utils.hpp +T002: Create src/network/PacketSerializer.hpp +T003: Add make_block() to tests/TestHelpers.hpp + +# Workstream B (Phase 3/US1): Peer operations +T004–T020: PeerManager vector→map migration + +# Workstream C (Phase 4/US2): RPC dispatch +T021–T034: RpcServer handler extraction + dispatch table +``` + +--- + +## Implementation Strategy + +### MVP First (User Story 1 Only) + +1. Complete Phase 2: Foundational (T001–T003) +2. Complete Phase 3: US1 — Peer Operations (T004–T020) +3. **STOP and VALIDATE**: Run `./tests/blockchain_tests` and `./tests/p2p_sync_integration_tests` +4. The highest-impact performance improvement is delivered + +### Incremental Delivery + +1. Phase 2 (Foundational) → shared utilities ready +2. Phase 3 (US1: Peer Ops) → O(1) peer lookups (**MVP**) +3. Phase 4 (US2: RPC Dispatch) → O(1) RPC dispatch + maintainability +4. Phase 5 (US3: Packet Serialize) → deduplication resolved +5. Phase 6 (US4: Test Helpers) → test code consolidated +6. Phase 7 (US5: Log Macros) → lazy formatting on hot paths +7. Phase 8 (Polish) → docs updated, full validation + +### Notes + +- [P] tasks = different files, no dependencies +- [Story] label maps task to specific user story for traceability +- Each user story is independently completable and testable +- Commit after each task or logical group +- Stop at any checkpoint to validate story independently +- Constitution requires: `make -j8`, individual test binary execution, roadmap update From d8f2c8183ac722ff7d913ffb5d9de173290d355d Mon Sep 17 00:00:00 2001 From: Tim Schwartz Date: Mon, 13 Apr 2026 15:43:07 -0500 Subject: [PATCH 4/4] Refactor RpcServer to use a dispatch mechanism for RPC handlers - Introduced a dispatch map to handle various RPC requests in RpcServer. - Added handler functions for multiple RPC methods including stream management and peer operations. - Updated the RpcServer header to include necessary includes and new member functions. Enhance logging utilities in utils.hpp - Added lazy log macros for different log levels to improve logging efficiency. - Updated the generate_uuid_v4 function declaration to ensure proper formatting. Improve test helpers for block creation and chain building - Added make_block function to TestHelpers for fast block creation with difficulty 0. - Updated chunk persistence tests to utilize new TestHelpers functions for directory management and block creation. Refactor consensus tests to use TestHelpers for block mining - Replaced inline block mining logic with TestHelpers::mineTestBlock for consistency and readability. - Updated various test cases to utilize the new helper functions for block creation. Refactor sync tests to use TestHelpers for block creation - Replaced inline block mining logic with TestHelpers::mineTestBlock and chain building with TestHelpers::buildValidChain. - Improved readability and maintainability of sync tests. Add serialization tests for packet headers and payloads - Implemented a test case to verify the correctness of packet serialization and deserialization for Block objects. --- docs/AUDIT.md | 84 +- docs/ROADMAP.md | 1 + specs/019-perf-dedup-cleanup/tasks.md | 8 +- src/BlockPropagation.cpp | 18 +- src/PeerManager.cpp | 179 ++-- src/PeerManager.hpp | 5 +- src/network/PacketSerializer.hpp | 28 + src/network/PeerClient.cpp | 62 +- src/network/PeerServer.cpp | 40 +- src/network/RpcServer.cpp | 1082 +++++++++++-------------- src/network/RpcServer.hpp | 28 + src/utils.hpp | 8 +- tests/TestHelpers.hpp | 18 + tests/chunk_persistence_tests.cpp | 90 +- tests/consensus_tests.cpp | 64 +- tests/sync_tests.cpp | 74 +- tests/utils_tests.cpp | 28 + 17 files changed, 838 insertions(+), 979 deletions(-) create mode 100644 src/network/PacketSerializer.hpp diff --git a/docs/AUDIT.md b/docs/AUDIT.md index 860d5ab..5092c0f 100644 --- a/docs/AUDIT.md +++ b/docs/AUDIT.md @@ -33,16 +33,18 @@ This audit found **4 bugs**, **2 security issues**, **5 performance issues**, test-quality weaknesses** that reduce confidence in the test suite. All 4 bugs, both security issues, and 1 performance issue were resolved in -**018-audit-bug-security-fixes**. Remaining items are tracked below. +**018-audit-bug-security-fixes**. Three more performance issues, both +duplication clusters, and 1 test-quality issue were resolved in +**019-perf-dedup-cleanup**. Remaining items are tracked below. | Category | Found | Resolved | Remaining | Highest Open Severity | |-----------------------|------:|---------:|----------:|----------------------:| | Bugs | 4 | 4 | 0 | — | | Security issues | 2 | 2 | 0 | — | -| Performance issues | 5 | 1 | 4 | Medium | -| Code duplication | 2 | 0 | 2 | Medium | +| Performance issues | 5 | 4 | 1 | Low | +| Code duplication | 2 | 2 | 0 | — | | Architecture concerns | 3 | 0 | 3 | Medium | -| Test quality issues | 6 | 0 | 6 | High | +| Test quality issues | 6 | 1 | 5 | High | --- @@ -91,39 +93,29 @@ returns JSON-RPC error -32001 "Block not found" for out-of-range indices. ## 4. Performance Issues -### 4.1 O(n) peer lookups — MEDIUM +### 4.1 ~~O(n) peer lookups — MEDIUM~~ ✅ RESOLVED (019) -`find_peer()`, `add_peer()`, `remove_peer()`, `is_banned()`, and -`get_non_banned_peer_addresses()` all perform linear scans over -`std::vector` and `std::vector`. +`peers_` and `bans_` are now `std::unordered_map` +keyed by `host:port`, giving O(1) lookups in `find_peer()`, `add_peer()`, +`remove_peer()`, `is_banned()`, `ban_peer()`, `unban_peer()`, and +`get_non_banned_peer_addresses()`. -With a configured maximum of 256 stored peers and `is_banned()` called on -every peer exchange, connection attempt, and block reception, switching to -`std::unordered_map` keyed by `host:port` would drop -lookups from O(n) to O(1). +### 4.2 ~~RPC dispatch is a 21-branch `if`/`else` chain — MEDIUM~~ ✅ RESOLVED (019) -### 4.2 RPC dispatch is a 21-branch `if`/`else` chain — MEDIUM - -[RpcServer.cpp](../src/network/RpcServer.cpp#L74-L704) - -Every request walks up to 21 string comparisons. A -`std::unordered_map` dispatch table would be O(1) and -reduce the 700-line `do_read()` callback into individually testable handler -functions. +`do_read()` now performs a single `dispatch_.find(method)` lookup into an +`std::unordered_map` initialized in `init_dispatch()`. +All 20 handlers are private methods returning `nlohmann::json`. ### 4.3 ~~`recoverChain()` loads each chunk multiple times — MEDIUM~~ ✅ RESOLVED (018) Resolved together with §2.2. Single-pass recovery loads each chunk once. -### 4.4 String construction in log calls — LOW - -Throughout the codebase, `logMessage("INFO", "Block #" + std::to_string(...) + ...)` -constructs the string even when the log level would suppress it. The -`logMessage()` function already filters by level, but the string allocation -happens at the call site. +### 4.4 ~~String construction in log calls — LOW~~ ✅ RESOLVED (019) -**Fix:** A level-check macro or a lazy-evaluation wrapper would eliminate -unnecessary allocations. +Hot-path `logMessage()` calls in `PeerManager.cpp`, `BlockPropagation.cpp`, +`PeerClient.cpp`, and `PeerServer.cpp` have been replaced with lazy +`LOG_INFO`/`LOG_WARN`/`LOG_ERROR`/`LOG_DEBUG` macros that check `getLogLevel()` +before evaluating the message expression. ### 4.5 `replaceChain()` loads entire candidate into memory — LOW @@ -137,19 +129,18 @@ simultaneously. A streaming/chunked replacement would bound memory usage. ## 5. Code Duplication -### 5.1 Packet serialization in PeerClient and PeerServer — MEDIUM +### 5.1 ~~Packet serialization in PeerClient and PeerServer — MEDIUM~~ ✅ RESOLVED (019) -`PeerClient::send()` ([PeerClient.cpp](../src/network/PeerClient.cpp#L352-L375)) and -`PeerServer::send_packet()` ([PeerServer.cpp](../src/network/PeerServer.cpp#L271-L300)) -share the same serialize → `PacketHeader` → `memcpy` → `async_write` pattern. -This could live in a shared utility or base class. +Both `PeerClient::send()` and `PeerServer::send_packet()` now call the +shared `serialize_packet()` template from `PacketSerializer.hpp`, which +returns header bytes + serialized payload. Each caller retains its own +`async_write` logic. -### 5.2 Test files still duplicate `mineTestBlock()` / `buildValidChain()` — LOW +### 5.2 ~~Test files still duplicate `mineTestBlock()` / `buildValidChain()` — LOW~~ ✅ RESOLVED (019) -Despite `TestHelpers.hpp` existing, `sync_tests.cpp`, -`block_propagation_tests.cpp`, `consensus_tests.cpp`, and -`chunk_persistence_tests.cpp` still define their own local versions of -`mineTestBlock()`, `buildValidChain()`, and temporary-directory helpers. +Local helper definitions in `sync_tests.cpp`, `consensus_tests.cpp`, and +`chunk_persistence_tests.cpp` have been removed. All test files now use the +shared `TestHelpers::` namespace. --- @@ -263,13 +254,12 @@ The following behaviors have no test coverage: | Block propagation relay excludes sender correctly | Medium | Open | | `recoverChain()` with corrupted index files (fallback to chunk rebuild) | Medium | Open | -### 7.6 Duplicated test setup persists in 4 files — LOW +### 7.6 ~~Duplicated test setup persists in 4 files — LOW~~ ✅ RESOLVED (019) -Despite `TestHelpers.hpp` existing, `sync_tests.cpp`, -`block_propagation_tests.cpp`, `consensus_tests.cpp`, and -`chunk_persistence_tests.cpp` still define local `mineTestBlock()` / -`buildValidChain()` / temp directory helpers instead of using the shared -utilities. +All local test helper definitions have been removed. Test files now use +`TestHelpers::mineTestBlock()`, `TestHelpers::buildValidChain()`, +`TestHelpers::createTestDir()`, `TestHelpers::cleanupTestDir()`, and +`TestHelpers::make_block()` exclusively. --- @@ -288,8 +278,8 @@ Ordered by impact and effort: | 5 | Rewrite `rpc_expansion_tests.cpp` to test real RPC handlers (§7.3) | False confidence → real coverage | Medium | Open | | 6 | Replace trivial assertions with meaningful ones (§7.1, §7.2) | Catches actual regressions | Medium | Open | | 7 | Cache chunk during `recoverChain()` validation (§2.2, §4.3) | 3× faster startup | Low | ✅ Done (018) | -| 8 | Replace O(n) peer lookups with `unordered_map` (§4.1) | O(1) peer operations | Medium | Open | -| 9 | Extract RPC dispatch table from `do_read()` (§4.2) | Maintainability, testability | Medium | Open | +| 8 | Replace O(n) peer lookups with `unordered_map` (§4.1) | O(1) peer operations | Medium | ✅ Done (019) | +| 9 | Extract RPC dispatch table from `do_read()` (§4.2) | Maintainability, testability | Medium | ✅ Done (019) | | 10 | Narrow `IBlockchain` into reader/writer interfaces (§6.1) | Reduces coupling | Medium | Open | -| 11 | Remove local test helpers in favor of `TestHelpers.hpp` (§7.6) | Consistency | Low | Open | +| 11 | Remove local test helpers in favor of `TestHelpers.hpp` (§7.6) | Consistency | Low | ✅ Done (019) | | 12 | Make integration tests deterministic (§7.4) | Reduces CI flakiness | Medium | Open | diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index d5ac7fa..6fa5d12 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -24,6 +24,7 @@ Last updated: 2026-04-13 | 016 | Code Audit Remediation | Fixed 5 bugs (block count, dirty flag, merkle root, IPv6 parsing, pending pool), cached difficulty per boundary with ChunkRetainGuard, extracted shared utilities (chunkFilename, parsePeerKey, RPC helpers, send_to_peers), added TestHelpers.hpp, single-threaded io_context enforcement | | 017 | Blockchain Module Split | Split monolithic Blockchain.cpp (1,019 lines) into four focused modules: ChainPersistence (379 lines), DifficultyEngine (95 lines), MerkleProofService (60 lines), and slimmed Blockchain core (624 lines); composition-based ownership; zero API changes; 3 new focused test suites | | 018 | Audit Bug & Security Fixes | Fixed 4 bugs (sync response block append, RPC getBlockByIndex bounds check, getBlockByIndex resize chunk IDs, recovery triple-load) and 2 security issues (port range validation in parsePeerKey, seed node input validation in main); single-pass recovery optimization | +| 019 | Performance & Deduplication Cleanup | O(1) peer lookups via `unordered_map`, RPC dispatch table replacing 21-branch `if`/`else`, shared `serialize_packet()` template, consolidated `TestHelpers`, lazy `LOG_*` macros | ## Suggested Specs diff --git a/specs/019-perf-dedup-cleanup/tasks.md b/specs/019-perf-dedup-cleanup/tasks.md index 1c97640..99c8109 100644 --- a/specs/019-perf-dedup-cleanup/tasks.md +++ b/specs/019-perf-dedup-cleanup/tasks.md @@ -148,10 +148,10 @@ _(No tasks)_ **Purpose**: Documentation updates, audit tracking, and validation. -- [ ] T047 [P] Update `docs/AUDIT.md` — mark §4.1 (O(n) peer lookups), §4.2 (RPC dispatch), §4.4 (log allocations), §5.1 (packet serialization), and §5.2 (test helpers) as ✅ RESOLVED with references to this feature (019) -- [ ] T048 [P] Update `docs/ROADMAP.md` — move 019-perf-dedup-cleanup from "Suggested Specs" or "In Progress" to "Completed" with one-line summary -- [ ] T049 Run quickstart.md verification checklist: confirm all 5 items pass -- [ ] T050 Final full build (`make -j8`) and run all test binaries individually to confirm zero regressions +- [X] T047 [P] Update `docs/AUDIT.md` — mark §4.1 (O(n) peer lookups), §4.2 (RPC dispatch), §4.4 (log allocations), §5.1 (packet serialization), and §5.2 (test helpers) as ✅ RESOLVED with references to this feature (019) +- [X] T048 [P] Update `docs/ROADMAP.md` — move 019-perf-dedup-cleanup from "Suggested Specs" or "In Progress" to "Completed" with one-line summary +- [X] T049 Run quickstart.md verification checklist: confirm all 5 items pass +- [X] T050 Final full build (`make -j8`) and run all test binaries individually to confirm zero regressions --- diff --git a/src/BlockPropagation.cpp b/src/BlockPropagation.cpp index ac12b4a..02e38b6 100644 --- a/src/BlockPropagation.cpp +++ b/src/BlockPropagation.cpp @@ -81,7 +81,7 @@ void BlockPropagation::defer_block(const Block &block, const std::string &sender pending_order_.push_back(key); } - logMessage("INFO", "Deferred block #" + std::to_string(block.index) + " waiting for predecessor"); + LOG_INFO("Deferred block #" + std::to_string(block.index) + " waiting for predecessor"); } void BlockPropagation::resolve_pending(const std::string &new_block_hash) @@ -95,7 +95,7 @@ void BlockPropagation::resolve_pending(const std::string &new_block_hash) if (order_it != pending_order_.end()) { pending_order_.erase(order_it); } - logMessage("INFO", "Resolving pending block #" + std::to_string(pb.block.index)); + LOG_INFO("Resolving pending block #" + std::to_string(pb.block.index)); on_block_received(pb.block, pb.sender_key); } } @@ -112,7 +112,7 @@ void BlockPropagation::evict_expired() continue; } if (now - it->second.inserted_at > kPendingTTL) { - logMessage("INFO", "Evicting expired pending block #" + std::to_string(it->second.block.index)); + LOG_INFO("Evicting expired pending block #" + std::to_string(it->second.block.index)); pending_map_.erase(it); pending_order_.pop_front(); } else { @@ -132,7 +132,7 @@ void BlockPropagation::appendReceivedBlock(const Block &block) block.merkleRoot, block.hash); bc_.appendBlock(verified); } catch (const std::invalid_argument &e) { - logMessage("WARN", "Block #" + std::to_string(block.index) + LOG_WARN("Block #" + std::to_string(block.index) + " rejected: " + std::string(e.what())); return; } @@ -146,7 +146,7 @@ void BlockPropagation::on_block_received(const Block &block, const std::string & { // Rate limit check if (!check_rate_limit(sender_key)) { - logMessage("WARN", "Rate limit exceeded for peer " + sender_key + ", dropping block #" + std::to_string(block.index)); + LOG_WARN("Rate limit exceeded for peer " + sender_key + ", dropping block #" + std::to_string(block.index)); if (peer_manager_) { try { auto [host, port] = parsePeerKey(sender_key); @@ -172,7 +172,7 @@ void BlockPropagation::on_block_received(const Block &block, const std::string & sync_queue_.pop_front(); } sync_queue_.emplace_back(block, sender_key); - logMessage("INFO", "Queued block #" + std::to_string(block.index) + " during sync"); + LOG_INFO("Queued block #" + std::to_string(block.index) + " during sync"); return; } @@ -190,7 +190,7 @@ void BlockPropagation::on_block_received(const Block &block, const std::string & // Validate against consensus const auto &config = bc_.getConfig(); if (!IBlockchain::isValidNewBlock(block, tip, config)) { - logMessage("WARN", "Invalid block #" + std::to_string(block.index) + " from " + sender_key); + LOG_WARN("Invalid block #" + std::to_string(block.index) + " from " + sender_key); if (peer_manager_) { try { auto [host, port] = parsePeerKey(sender_key); @@ -203,7 +203,7 @@ void BlockPropagation::on_block_received(const Block &block, const std::string & // Append valid block appendReceivedBlock(block); - logMessage("INFO", "Block #" + std::to_string(block.index) + " validated and appended"); + LOG_INFO("Block #" + std::to_string(block.index) + " validated and appended"); // Add to dedup cache cache_insert(block.hash); @@ -221,7 +221,7 @@ void BlockPropagation::on_block_received(const Block &block, const std::string & void BlockPropagation::process_sync_queue() { - logMessage("INFO", "Processing sync queue (" + std::to_string(sync_queue_.size()) + " blocks)"); + LOG_INFO("Processing sync queue (" + std::to_string(sync_queue_.size()) + " blocks)"); auto queue = std::move(sync_queue_); sync_queue_.clear(); diff --git a/src/PeerManager.cpp b/src/PeerManager.cpp index b6ffe54..d6b279b 100644 --- a/src/PeerManager.cpp +++ b/src/PeerManager.cpp @@ -38,7 +38,7 @@ void PeerManager::load_peers() { auto peers_path = data_dir_ / "peers.json"; if (!std::filesystem::exists(peers_path)) { node_uuid_ = generate_uuid_v4(); - logMessage("INFO", "Generated new node UUID: " + node_uuid_); + LOG_INFO("Generated new node UUID: " + node_uuid_); save_peers(); return; } @@ -46,7 +46,7 @@ void PeerManager::load_peers() { std::ifstream ifs(peers_path); if (!ifs.is_open()) { node_uuid_ = generate_uuid_v4(); - logMessage("WARN", "Cannot open peers.json, starting fresh with UUID: " + node_uuid_); + LOG_WARN("Cannot open peers.json, starting fresh with UUID: " + node_uuid_); save_peers(); return; } @@ -55,7 +55,7 @@ void PeerManager::load_peers() { try { j = nlohmann::json::parse(ifs); } catch (const nlohmann::json::parse_error &e) { - logMessage("WARN", "Malformed peers.json, starting with empty peer list: " + std::string(e.what())); + LOG_WARN("Malformed peers.json, starting with empty peer list: " + std::string(e.what())); node_uuid_ = generate_uuid_v4(); save_peers(); return; @@ -68,14 +68,20 @@ void PeerManager::load_peers() { } if (j.contains("peers") && j["peers"].is_array()) { - peers_ = j["peers"].get>(); + auto peer_vec = j["peers"].get>(); + for (auto &p : peer_vec) { + peers_[peer_key(p.host, p.port)] = std::move(p); + } } if (j.contains("bans") && j["bans"].is_array()) { - bans_ = j["bans"].get>(); + auto ban_vec = j["bans"].get>(); + for (auto &b : ban_vec) { + bans_[peer_key(b.host, b.port)] = std::move(b); + } } - logMessage("INFO", "Loaded " + std::to_string(peers_.size()) + " peers, UUID: " + node_uuid_); + LOG_INFO("Loaded " + std::to_string(peers_.size()) + " peers, UUID: " + node_uuid_); } void PeerManager::save_peers() { @@ -84,12 +90,34 @@ void PeerManager::save_peers() { nlohmann::json j; j["node_uuid"] = node_uuid_; - j["peers"] = peers_; - j["bans"] = bans_; + + // Serialize map values as JSON arrays to preserve on-disk format + nlohmann::json peers_arr = nlohmann::json::array(); + for (const auto &[key, entry] : peers_) { + nlohmann::json pj; + pj["host"] = entry.host; + pj["port"] = entry.port; + pj["node_uuid"] = entry.node_uuid; + pj["last_seen"] = entry.last_seen; + pj["error_count"] = entry.error_count; + peers_arr.push_back(pj); + } + j["peers"] = peers_arr; + + nlohmann::json bans_arr = nlohmann::json::array(); + for (const auto &[key, ban] : bans_) { + nlohmann::json bj; + bj["host"] = ban.host; + bj["port"] = ban.port; + bj["reason"] = ban.reason; + bj["expires"] = ban.expires; + bans_arr.push_back(bj); + } + j["bans"] = bans_arr; std::ofstream ofs(temp_path); if (!ofs.is_open()) { - logMessage("ERROR", "Cannot write peers.json.tmp"); + LOG_ERROR("Cannot write peers.json.tmp"); return; } ofs << j.dump(2) << std::endl; @@ -98,7 +126,7 @@ void PeerManager::save_peers() { std::error_code ec; std::filesystem::rename(temp_path, peers_path, ec); if (ec) { - logMessage("ERROR", "Failed to rename peers.json.tmp: " + ec.message()); + LOG_ERROR("Failed to rename peers.json.tmp: " + ec.message()); } } @@ -118,14 +146,14 @@ static std::string normalize_address(const std::string &host) { bool PeerManager::add_peer(const PeerEntry &entry) { auto norm_host = normalize_address(entry.host); - // Check if already exists - for (auto &p : peers_) { - if (p.host == norm_host && p.port == entry.port) { - // Update existing entry - if (!entry.node_uuid.empty()) p.node_uuid = entry.node_uuid; - if (entry.last_seen > p.last_seen) p.last_seen = entry.last_seen; - return true; - } + auto key = peer_key(norm_host, entry.port); + + auto it = peers_.find(key); + if (it != peers_.end()) { + // Update existing entry + if (!entry.node_uuid.empty()) it->second.node_uuid = entry.node_uuid; + if (entry.last_seen > it->second.last_seen) it->second.last_seen = entry.last_seen; + return true; } // Cap enforcement with oldest-seen eviction @@ -133,49 +161,50 @@ bool PeerManager::add_peer(const PeerEntry &entry) { evict_oldest_peer(); } - peers_.push_back(entry); - peers_.back().host = norm_host; + PeerEntry new_entry = entry; + new_entry.host = norm_host; + peers_[key] = std::move(new_entry); return true; } void PeerManager::evict_oldest_peer() { if (peers_.empty()) return; - auto oldest = std::min_element(peers_.begin(), peers_.end(), - [](const PeerEntry &a, const PeerEntry &b) { - return a.last_seen < b.last_seen; - }); + auto oldest = peers_.begin(); + for (auto it = peers_.begin(); it != peers_.end(); ++it) { + if (it->second.last_seen < oldest->second.last_seen) { + oldest = it; + } + } peers_.erase(oldest); } bool PeerManager::remove_peer(const std::string &host_raw, uint16_t port) { auto host = normalize_address(host_raw); - auto it = std::remove_if(peers_.begin(), peers_.end(), - [&](const PeerEntry &p) { return p.host == host && p.port == port; }); - if (it != peers_.end()) { - peers_.erase(it, peers_.end()); - return true; - } - return false; + auto key = peer_key(host, port); + return peers_.erase(key) > 0; } std::vector PeerManager::get_peers() const { - return peers_; + std::vector result; + result.reserve(peers_.size()); + for (const auto &[key, entry] : peers_) { + result.push_back(entry); + } + return result; } PeerEntry* PeerManager::find_peer(const std::string &host_raw, uint16_t port) { auto host = normalize_address(host_raw); - for (auto &p : peers_) { - if (p.host == host && p.port == port) return &p; - } + auto it = peers_.find(peer_key(host, port)); + if (it != peers_.end()) return &it->second; return nullptr; } const PeerEntry* PeerManager::find_peer(const std::string &host_raw, uint16_t port) const { auto host = normalize_address(host_raw); - for (auto &p : peers_) { - if (p.host == host && p.port == port) return &p; - } + auto it = peers_.find(peer_key(host, port)); + if (it != peers_.end()) return &it->second; return nullptr; } @@ -216,11 +245,10 @@ void PeerManager::start() { } // Also connect to known peers from peers.json - for (const auto &peer : peers_) { + for (const auto &[pk, peer] : peers_) { if (outbound_connections_.size() >= config_.max_outbound) break; if (is_banned(peer.host, peer.port)) continue; - auto key = peer_key(peer.host, peer.port); - if (outbound_connections_.count(key)) continue; + if (outbound_connections_.count(pk)) continue; connect_to(peer.host, peer.port); } @@ -234,7 +262,7 @@ void PeerManager::connect_to(const std::string &host_raw, uint16_t port) { // Check limits if (outbound_connections_.size() >= config_.max_outbound) { - logMessage("WARN", "Outbound connection limit reached, cannot connect to " + key); + LOG_WARN("Outbound connection limit reached, cannot connect to " + key); return; } @@ -245,17 +273,17 @@ void PeerManager::connect_to(const std::string &host_raw, uint16_t port) { // Check ban if (is_banned(host, port)) { - logMessage("WARN", "Peer " + key + " is banned, skipping connection"); + LOG_WARN("Peer " + key + " is banned, skipping connection"); return; } // Never connect to ourselves if (is_self(host, port)) { - logMessage("DEBUG", "Skipping self-connection to " + key); + LOG_DEBUG("Skipping self-connection to " + key); return; } - logMessage("INFO", "Connecting to peer " + key); + LOG_INFO("Connecting to peer " + key); auto client = std::make_shared(io_context_, ssl_context_, host, port, bc_, sync_status_); client->set_peer_manager(this); @@ -282,7 +310,7 @@ void PeerManager::on_peer_disconnected(const std::string &host_ref, uint16_t por peer->error_count++; } - logMessage("INFO", "Peer disconnected: " + key); + LOG_INFO("Peer disconnected: " + key); // Check if we still have an inbound session from the same node (e.g. dedup dropped // our outbound but the inbound is still alive). If so, skip reconnect. @@ -304,8 +332,7 @@ void PeerManager::on_peer_disconnected(const std::string &host_ref, uint16_t por // Try to replace with a *different* known peer if (config_.discovery_enabled && outbound_connections_.size() < config_.max_outbound) { - for (const auto &p : peers_) { - auto pk = peer_key(p.host, p.port); + for (const auto &[pk, p] : peers_) { if (pk == key) continue; // Skip the peer that just disconnected if (outbound_connections_.count(pk)) continue; if (is_banned(p.host, p.port)) continue; @@ -321,7 +348,7 @@ void PeerManager::on_inbound_connected(const std::string &host_raw, uint16_t por auto key = peer_key(host, port); inbound_sessions_[key] = session; inbound_count_++; - logMessage("INFO", "Inbound connection from " + key + " (total: " + std::to_string(inbound_count_) + ")"); + LOG_INFO("Inbound connection from " + key + " (total: " + std::to_string(inbound_count_) + ")"); } void PeerManager::on_inbound_disconnected(const std::string &host_raw, uint16_t port) { @@ -329,7 +356,7 @@ void PeerManager::on_inbound_disconnected(const std::string &host_raw, uint16_t auto key = peer_key(host, port); inbound_sessions_.erase(key); if (inbound_count_ > 0) inbound_count_--; - logMessage("INFO", "Inbound disconnection from " + key + " (total: " + std::to_string(inbound_count_) + ")"); + LOG_INFO("Inbound disconnection from " + key + " (total: " + std::to_string(inbound_count_) + ")"); } // --- Peer Exchange --- @@ -420,7 +447,7 @@ void PeerManager::check_duplicate_connection(const std::string &remote_uuid, // If the remote UUID is our own, it's a self-connection — drop immediately if (remote_uuid == node_uuid_) { - logMessage("WARN", "Self-connection detected (UUID " + remote_uuid + ") — closing"); + LOG_WARN("Self-connection detected (UUID " + remote_uuid + ") — closing"); auto self_key = peer_key(host, port); // Defer the erase so the calling PeerClient is not destroyed mid-call boost::asio::post(io_context_, [this, self_key]() { @@ -456,10 +483,10 @@ void PeerManager::check_duplicate_connection(const std::string &remote_uuid, // The dedup rule: lower UUID keeps its outbound if (node_uuid_ < remote_uuid) { - logMessage("INFO", "Duplicate connection detected for UUID " + remote_uuid + " — keeping our outbound (lower UUID)"); + LOG_INFO("Duplicate connection detected for UUID " + remote_uuid + " — keeping our outbound (lower UUID)"); // The remote (higher UUID) should drop its outbound to us; nothing to do here } else { - logMessage("INFO", "Duplicate connection detected for UUID " + remote_uuid + " — dropping our outbound (higher UUID)"); + LOG_INFO("Duplicate connection detected for UUID " + remote_uuid + " — dropping our outbound (higher UUID)"); // Drop our outbound connection; keep the inbound boost::asio::post(io_context_, [this, dup_outbound_key]() { outbound_connections_.erase(dup_outbound_key); @@ -504,45 +531,41 @@ void PeerManager::ban_peer(const std::string &host, uint16_t port, const std::st // Remove existing ban for same address unban_peer(host, port); - bans_.push_back(ban); + bans_[peer_key(host, port)] = ban; save_peers(); - logMessage("INFO", "Banned peer " + key + " reason: " + reason); + LOG_INFO("Banned peer " + key + " reason: " + reason); } void PeerManager::unban_peer(const std::string &host, uint16_t port) { - bans_.erase( - std::remove_if(bans_.begin(), bans_.end(), - [&](const BanRecord &b) { return b.host == host && b.port == port; }), - bans_.end() - ); + bans_.erase(peer_key(host, port)); } bool PeerManager::is_banned(const std::string &host, uint16_t port) const { + auto it = bans_.find(peer_key(host, port)); + if (it == bans_.end()) return false; auto now = static_cast(std::time(nullptr)); - for (const auto &ban : bans_) { - if (ban.host == host && ban.port == port) { - if (ban.expires == 0 || ban.expires > now) { - return true; - } - } - } - return false; + return it->second.expires == 0 || it->second.expires > now; } void PeerManager::purge_expired_bans() { auto now = static_cast(std::time(nullptr)); - bans_.erase( - std::remove_if(bans_.begin(), bans_.end(), - [now](const BanRecord &b) { - return b.expires > 0 && b.expires <= now; - }), - bans_.end() - ); + for (auto it = bans_.begin(); it != bans_.end(); ) { + if (it->second.expires > 0 && it->second.expires <= now) { + it = bans_.erase(it); + } else { + ++it; + } + } } std::vector PeerManager::get_bans() const { - return bans_; + std::vector result; + result.reserve(bans_.size()); + for (const auto &[key, ban] : bans_) { + result.push_back(ban); + } + return result; } // --- Reconnection with Backoff --- @@ -563,7 +586,7 @@ void PeerManager::schedule_reconnect(const std::string &host, uint16_t port) { double jitter_factor = 0.8 + (std::uniform_real_distribution(0.0, 0.4)(gen)); auto delay_ms = static_cast(state.current_delay * jitter_factor * 1000); - logMessage("INFO", "Scheduling reconnect to " + key + " in " + std::to_string(delay_ms / 1000) + "s"); + LOG_INFO("Scheduling reconnect to " + key + " in " + std::to_string(delay_ms / 1000) + "s"); state.timer = std::make_shared(io_context_); state.timer->expires_after(std::chrono::milliseconds(delay_ms)); @@ -602,7 +625,7 @@ uint16_t PeerManager::get_listen_port() const { std::vector PeerManager::get_non_banned_peer_addresses() const { std::vector addresses; - for (const auto &p : peers_) { + for (const auto &[key, p] : peers_) { if (!is_banned(p.host, p.port)) { addresses.push_back({p.host, p.port}); } diff --git a/src/PeerManager.hpp b/src/PeerManager.hpp index 532ac0f..2ce620c 100644 --- a/src/PeerManager.hpp +++ b/src/PeerManager.hpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -111,8 +112,8 @@ class PeerManager { uint16_t p2p_port_; std::string node_uuid_; - std::vector peers_; - std::vector bans_; + std::unordered_map peers_; + std::unordered_map bans_; // Connection tracking std::map> outbound_connections_; // "host:port" -> PeerClient diff --git a/src/network/PacketSerializer.hpp b/src/network/PacketSerializer.hpp new file mode 100644 index 0000000..2d38e33 --- /dev/null +++ b/src/network/PacketSerializer.hpp @@ -0,0 +1,28 @@ +#pragma once + +#include "PacketHeader.hpp" +#include +#include +#include +#include +#include +#include + +// Shared packet serialization utility. +// Returns (header_bytes, serialized_payload) for callers to write via their own async_write. +template +std::pair, std::string> serialize_packet(const T &obj, uint64_t packet_type) +{ + std::stringstream ss; + { + boost::archive::binary_oarchive oa(ss); + oa << obj; + } + std::string serialized = ss.str(); + + PacketHeader header(serialized.size(), packet_type); + std::vector header_data(sizeof(header)); + std::memcpy(header_data.data(), &header, sizeof(header)); + + return {std::move(header_data), std::move(serialized)}; +} diff --git a/src/network/PeerClient.cpp b/src/network/PeerClient.cpp index 29fd9b6..52915f8 100644 --- a/src/network/PeerClient.cpp +++ b/src/network/PeerClient.cpp @@ -1,5 +1,6 @@ #include "PeerClient.hpp" #include "PacketHeader.hpp" +#include "PacketSerializer.hpp" #include "SyncMessages.hpp" #include "PeerMessages.hpp" #include "../PeerManager.hpp" @@ -28,7 +29,7 @@ void PeerClient::connect() { if (!ec) { - logMessage("INFO", "Connected to peer " + host + ":" + port); + LOG_INFO("Connected to peer " + host + ":" + port); connected = true; // Reset reconnect backoff on successful connection if (peer_manager) { @@ -41,14 +42,14 @@ void PeerClient::connect() } else { - logMessage("ERROR", "TLS handshake failed: " + ec.message()); + LOG_ERROR("TLS handshake failed: " + ec.message()); handle_disconnect("TLS handshake failed"); } }); } else { - logMessage("ERROR", "Connection to peer failed: " + ec.message()); + LOG_ERROR("Connection to peer failed: " + ec.message()); handle_disconnect("Connection failed"); } }); @@ -63,19 +64,19 @@ void PeerClient::send_peer_exchange() req.sender_listen_port = peer_manager->get_listen_port(); req.peers = peer_manager->get_non_banned_peer_addresses(); - logMessage("INFO", "Sending PEER_EXCHANGE to " + host + ":" + port + " with " + std::to_string(req.peers.size()) + " peers"); + LOG_INFO("Sending PEER_EXCHANGE to " + host + ":" + port + " with " + std::to_string(req.peers.size()) + " peers"); send(req, PacketType::PEER_EXCHANGE); } void PeerClient::start_sync() { if (sync_status.isSyncing.load()) { - logMessage("WARN", "Sync already in progress, skipping"); + LOG_WARN("Sync already in progress, skipping"); return; } sync_status.isSyncing.store(true); - logMessage("INFO", "Starting chain sync with peer"); + LOG_INFO("Starting chain sync with peer"); send_sync_query(); } @@ -83,7 +84,7 @@ void PeerClient::send_sync_query() { SyncQuery query; query.local_chain_height = bc.getChainBlockCount(); - logMessage("INFO", "Sending BLOCKCHAIN_QUERY with local_chain_height=" + std::to_string(query.local_chain_height)); + LOG_INFO("Sending BLOCKCHAIN_QUERY with local_chain_height=" + std::to_string(query.local_chain_height)); send(query, PacketType::BLOCKCHAIN_QUERY); @@ -138,13 +139,13 @@ void PeerClient::do_read_body(const PacketHeader &header) boost::archive::binary_iarchive ia(iss); Block b; ia >> b; - logMessage("INFO", "Received block #" + std::to_string(b.index)); + LOG_INFO("Received block #" + std::to_string(b.index)); if (block_propagation_) { auto sender_key = host + ":" + port; block_propagation_->on_block_received(b, sender_key); } } catch (const std::exception &e) { - logMessage("ERROR", "Failed to deserialize BLOCK: " + std::string(e.what())); + LOG_ERROR("Failed to deserialize BLOCK: " + std::string(e.what())); if (peer_manager) { peer_manager->increment_error(host, static_cast(std::stoi(port))); } @@ -160,7 +161,7 @@ void PeerClient::do_read_body(const PacketHeader &header) ia >> response; handle_peer_exchange_response(response); } catch (const std::exception &e) { - logMessage("ERROR", "Failed to deserialize PEER_EXCHANGE_RESPONSE: " + std::string(e.what())); + LOG_ERROR("Failed to deserialize PEER_EXCHANGE_RESPONSE: " + std::string(e.what())); if (peer_manager) { peer_manager->increment_error(host, static_cast(std::stoi(port))); } @@ -180,7 +181,7 @@ void PeerClient::do_read_body(const PacketHeader &header) peer_manager->on_peer_exchange_received(req.sender_uuid, req.sender_listen_port, host, req.peers); } } catch (const std::exception &e) { - logMessage("ERROR", "Failed to deserialize PEER_EXCHANGE: " + std::string(e.what())); + LOG_ERROR("Failed to deserialize PEER_EXCHANGE: " + std::string(e.what())); if (peer_manager) { peer_manager->increment_error(host, static_cast(std::stoi(port))); } @@ -189,7 +190,7 @@ void PeerClient::do_read_body(const PacketHeader &header) return; } default: - logMessage("WARN", "Received unknown packet type: " + std::to_string(header.type)); + LOG_WARN("Received unknown packet type: " + std::to_string(header.type)); break; } } else { @@ -202,7 +203,7 @@ void PeerClient::do_read_body(const PacketHeader &header) void PeerClient::handle_sync_response(const SyncResponse &response) { - logMessage("INFO", "Received BLOCKCHAIN_RESPONSE: chunk=" + std::to_string(response.chunk_index) + LOG_INFO("Received BLOCKCHAIN_RESPONSE: chunk=" + std::to_string(response.chunk_index) + " blocks=" + std::to_string(response.blocks.size()) + " total_chain_height=" + std::to_string(response.total_chain_height)); @@ -210,7 +211,7 @@ void PeerClient::handle_sync_response(const SyncResponse &response) // Longest-chain guard: skip sync if peer is not strictly longer if (response.total_chain_height <= local_height) { - logMessage("INFO", "Peer chain height (" + std::to_string(response.total_chain_height) + LOG_INFO("Peer chain height (" + std::to_string(response.total_chain_height) + ") is not longer than local (" + std::to_string(local_height) + "), sync not needed"); sync_status.isSyncing.store(false); return; @@ -219,10 +220,10 @@ void PeerClient::handle_sync_response(const SyncResponse &response) // Empty response while expecting more blocks signals end-of-sync if (response.blocks.empty()) { if (response.total_chain_height > local_height) { - logMessage("WARN", "Empty sync response while expecting more blocks (local=" + LOG_WARN("Empty sync response while expecting more blocks (local=" + std::to_string(local_height) + " peer=" + std::to_string(response.total_chain_height) + ")"); } else { - logMessage("INFO", "Received empty sync response, chain is up to date"); + LOG_INFO("Received empty sync response, chain is up to date"); } sync_status.isSyncing.store(false); return; @@ -245,7 +246,7 @@ void PeerClient::handle_sync_response(const SyncResponse &response) if (block.index > 0 && block.index - 1 < local_height) { prev_block = bc.getBlockByIndex(block.index - 1); } else { - logMessage("ERROR", "Cannot validate block " + std::to_string(block.index) + LOG_ERROR("Cannot validate block " + std::to_string(block.index) + ": no previous block available"); abort_sync("Missing previous block for validation at index " + std::to_string(block.index)); return; @@ -255,7 +256,7 @@ void PeerClient::handle_sync_response(const SyncResponse &response) } if (!IBlockchain::isValidNewBlock(block, prev_block, config)) { - logMessage("ERROR", "Block " + std::to_string(block.index) + LOG_ERROR("Block " + std::to_string(block.index) + " failed validation in chunk " + std::to_string(response.chunk_index) + " from peer " + host + ":" + port); abort_sync("Invalid block at index " + std::to_string(block.index)); @@ -263,7 +264,7 @@ void PeerClient::handle_sync_response(const SyncResponse &response) } } - logMessage("INFO", "Chunk " + std::to_string(response.chunk_index) + " validated successfully (" + LOG_INFO("Chunk " + std::to_string(response.chunk_index) + " validated successfully (" + std::to_string(response.blocks.size()) + " blocks)"); // Persist the valid chunk: append blocks to the chain @@ -290,7 +291,7 @@ void PeerClient::handle_sync_response(const SyncResponse &response) } size_t new_local_height = bc.getChainBlockCount(); - logMessage("INFO", "Synced " + std::to_string(response.blocks.size()) + LOG_INFO("Synced " + std::to_string(response.blocks.size()) + " blocks, local height now " + std::to_string(new_local_height)); // Check if we need more chunks @@ -299,7 +300,7 @@ void PeerClient::handle_sync_response(const SyncResponse &response) arm_chunk_timer(); do_read_header(); } else { - logMessage("INFO", "Chain sync complete, local height=" + std::to_string(new_local_height)); + LOG_INFO("Chain sync complete, local height=" + std::to_string(new_local_height)); sync_status.isSyncing.store(false); if (block_propagation_) { block_propagation_->process_sync_queue(); @@ -309,7 +310,7 @@ void PeerClient::handle_sync_response(const SyncResponse &response) void PeerClient::abort_sync(const std::string &reason) { - logMessage("ERROR", "Sync aborted: " + reason); + LOG_ERROR("Sync aborted: " + reason); cancel_chunk_timer(); sync_status.isSyncing.store(false); } @@ -317,7 +318,7 @@ void PeerClient::abort_sync(const std::string &reason) void PeerClient::handle_peer_exchange_response(const PeerExchangeResponse &response) { remote_uuid_ = response.sender_uuid; - logMessage("INFO", "Received PEER_EXCHANGE_RESPONSE from " + host + ":" + port + LOG_INFO("Received PEER_EXCHANGE_RESPONSE from " + host + ":" + port + " uuid=" + response.sender_uuid + " peers=" + std::to_string(response.peers.size())); if (peer_manager) { @@ -330,7 +331,7 @@ void PeerClient::handle_disconnect(const std::string &reason) { if (!connected && !socket.lowest_layer().is_open()) return; connected = false; - logMessage("INFO", "Peer " + host + ":" + port + " disconnected: " + reason); + LOG_INFO("Peer " + host + ":" + port + " disconnected: " + reason); boost::system::error_code ec; socket.lowest_layer().close(ec); @@ -361,14 +362,7 @@ void PeerClient::cancel_chunk_timer() template void PeerClient::send(const T &obj, uint64_t packet_type) { - std::stringstream ss; - boost::archive::binary_oarchive oa(ss); - oa << obj; - std::string serialized_str = ss.str(); - - PacketHeader header(serialized_str.size(), packet_type); - std::vector header_data(sizeof(header)); - std::memcpy(header_data.data(), &header, sizeof(header)); + auto [header_data, serialized_str] = serialize_packet(obj, packet_type); std::ostream stream(&this->write_buffer); stream.write(header_data.data(), header_data.size()); @@ -377,9 +371,9 @@ void PeerClient::send(const T &obj, uint64_t packet_type) boost::asio::async_write(this->socket, this->write_buffer, [this](const boost::system::error_code &ec, std::size_t) { if (!ec) { - logMessage("INFO", "Packet sent successfully"); + LOG_INFO("Packet sent successfully"); } else { - logMessage("ERROR", "Error sending packet: " + ec.message()); + LOG_ERROR("Error sending packet: " + ec.message()); } }); } diff --git a/src/network/PeerServer.cpp b/src/network/PeerServer.cpp index 38b0f8a..059d250 100644 --- a/src/network/PeerServer.cpp +++ b/src/network/PeerServer.cpp @@ -4,6 +4,7 @@ #include "../BlockPropagation.hpp" #include "SyncMessages.hpp" #include "PeerMessages.hpp" +#include "PacketSerializer.hpp" #include #include @@ -25,7 +26,7 @@ void PeerServer::on_handshake_complete() { // Check if inbound connection can be accepted if (peer_manager && !peer_manager->can_accept_inbound()) { - logMessage("WARN", "Inbound connection limit reached, rejecting"); + LOG_WARN("Inbound connection limit reached, rejecting"); boost::system::error_code ec; ssl_socket.lowest_layer().close(ec); return; @@ -36,7 +37,7 @@ void PeerServer::on_handshake_complete() auto rhost = remote_host(); auto rport = remote_port(); if (peer_manager->is_banned(rhost, rport)) { - logMessage("WARN", "Rejecting banned peer " + rhost + ":" + std::to_string(rport)); + LOG_WARN("Rejecting banned peer " + rhost + ":" + std::to_string(rport)); boost::system::error_code ec; ssl_socket.lowest_layer().close(ec); return; @@ -84,13 +85,13 @@ void PeerServer::do_read_body(const PacketHeader &header) boost::archive::binary_iarchive ia(iss); Block b; ia >> b; - logMessage("INFO", "Received block #" + std::to_string(b.index)); + LOG_INFO("Received block #" + std::to_string(b.index)); if (block_propagation_) { auto sender_key = remote_host() + ":" + std::to_string(remote_port()); block_propagation_->on_block_received(b, sender_key); } } catch (const std::exception &e) { - logMessage("ERROR", "Failed to deserialize BLOCK: " + std::string(e.what())); + LOG_ERROR("Failed to deserialize BLOCK: " + std::string(e.what())); if (peer_manager) { peer_manager->increment_error(remote_host(), remote_port()); } @@ -102,7 +103,7 @@ void PeerServer::do_read_body(const PacketHeader &header) boost::archive::binary_iarchive ia(iss); SyncQuery query; ia >> query; - logMessage("INFO", "Received BLOCKCHAIN_QUERY with local_chain_height=" + std::to_string(query.local_chain_height)); + LOG_INFO("Received BLOCKCHAIN_QUERY with local_chain_height=" + std::to_string(query.local_chain_height)); handle_blockchain_query(query); return; // handler chains back to do_read_header } @@ -112,11 +113,11 @@ void PeerServer::do_read_body(const PacketHeader &header) boost::archive::binary_iarchive ia(iss); PeerExchangeRequest request; ia >> request; - logMessage("INFO", "Received PEER_EXCHANGE from uuid=" + request.sender_uuid + LOG_INFO("Received PEER_EXCHANGE from uuid=" + request.sender_uuid + " peers=" + std::to_string(request.peers.size())); handle_peer_exchange(request); } catch (const std::exception &e) { - logMessage("ERROR", "Failed to deserialize PEER_EXCHANGE: " + std::string(e.what())); + LOG_ERROR("Failed to deserialize PEER_EXCHANGE: " + std::string(e.what())); if (peer_manager) { peer_manager->increment_error(remote_host(), remote_port()); } @@ -129,13 +130,13 @@ void PeerServer::do_read_body(const PacketHeader &header) boost::archive::binary_iarchive ia(iss); PeerExchangeResponse response; ia >> response; - logMessage("INFO", "Received PEER_EXCHANGE_RESPONSE from uuid=" + response.sender_uuid); + LOG_INFO("Received PEER_EXCHANGE_RESPONSE from uuid=" + response.sender_uuid); if (peer_manager) { peer_manager->on_peer_exchange_received(response.sender_uuid, response.sender_listen_port, remote_host(), response.peers); } } catch (const std::exception &e) { - logMessage("ERROR", "Failed to deserialize PEER_EXCHANGE_RESPONSE: " + std::string(e.what())); + LOG_ERROR("Failed to deserialize PEER_EXCHANGE_RESPONSE: " + std::string(e.what())); if (peer_manager) { peer_manager->increment_error(remote_host(), remote_port()); } @@ -143,13 +144,13 @@ void PeerServer::do_read_body(const PacketHeader &header) break; } default: - logMessage("ERROR", "Received unknown packet type: " + std::to_string(header.type)); + LOG_ERROR("Received unknown packet type: " + std::to_string(header.type)); break; } do_read_header(); } else { - logMessage("ERROR", "Read body failed: " + ec.message()); + LOG_ERROR("Read body failed: " + ec.message()); if (peer_manager) { peer_manager->increment_error(remote_host(), remote_port()); peer_manager->on_inbound_disconnected(remote_host(), remote_port()); @@ -214,7 +215,7 @@ void PeerServer::send_sync_response(const SyncResponse &response, size_t remaini boost::asio::async_write(this->ssl_socket, buffers, [this, self, header_buf, payload_buf, remaining_chunks, next_chunk, total_height](const boost::system::error_code &ec, std::size_t) { if (!ec) { - logMessage("INFO", "Sent BLOCKCHAIN_RESPONSE chunk " + std::to_string(next_chunk > 0 ? next_chunk - 1 : 0)); + LOG_INFO("Sent BLOCKCHAIN_RESPONSE chunk " + std::to_string(next_chunk > 0 ? next_chunk - 1 : 0)); if (remaining_chunks > 0) { // Build and send the next chunk @@ -235,7 +236,7 @@ void PeerServer::send_sync_response(const SyncResponse &response, size_t remaini do_read_header(); } } else { - logMessage("ERROR", "Failed to send BLOCKCHAIN_RESPONSE: " + ec.message()); + LOG_ERROR("Failed to send BLOCKCHAIN_RESPONSE: " + ec.message()); } }); } @@ -273,14 +274,9 @@ void PeerServer::send_packet(const T &obj, uint64_t packet_type) { auto self(shared_from_this()); - std::stringstream ss; - boost::archive::binary_oarchive oa(ss); - oa << obj; - std::string serialized = ss.str(); + auto [header_data, serialized] = serialize_packet(obj, packet_type); - PacketHeader header(serialized.size(), packet_type); - auto header_buf = std::make_shared>(sizeof(header)); - std::memcpy(header_buf->data(), &header, sizeof(header)); + auto header_buf = std::make_shared>(std::move(header_data)); auto payload_buf = std::make_shared(std::move(serialized)); std::array buffers = { @@ -291,10 +287,10 @@ void PeerServer::send_packet(const T &obj, uint64_t packet_type) boost::asio::async_write(this->ssl_socket, buffers, [this, self, header_buf, payload_buf, packet_type](const boost::system::error_code &ec, std::size_t) { if (!ec) { - logMessage("INFO", "Sent packet type " + std::to_string(packet_type)); + LOG_INFO("Sent packet type " + std::to_string(packet_type)); do_read_header(); } else { - logMessage("ERROR", "Failed to send packet: " + ec.message()); + LOG_ERROR("Failed to send packet: " + ec.message()); } }); } diff --git a/src/network/RpcServer.cpp b/src/network/RpcServer.cpp index b4f08e1..405b8de 100644 --- a/src/network/RpcServer.cpp +++ b/src/network/RpcServer.cpp @@ -11,7 +11,7 @@ #include RpcServer::RpcServer(std::shared_ptr> socket_ptr, IBlockchain &bc) - : SessionHandler(std::move(*socket_ptr), bc) {} + : SessionHandler(std::move(*socket_ptr), bc) { init_dispatch(); } std::shared_ptr RpcServer::create(boost::asio::io_context &io_context, ssl::context &ssl_context, IBlockchain &bc) { @@ -71,646 +71,488 @@ void RpcServer::do_read() return; } - if(object["method"] == "publish") - { - // Gate publish during sync - if (sync_status && sync_status->isSyncing.load()) { - buffer.consume(buffer.size()); - outputStream << syncInProgressMessage(object["id"]) << std::endl; - this->do_write(); - return; - } - - if (object["params"] == nullptr || object["params"].type() != nlohmann::json::value_t::object) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params") << std::endl; - this->do_write(); - return; - } - - // Validate stream param - if (!object["params"].contains("stream") || !object["params"]["stream"].is_string() - || object["params"]["stream"].get().empty()) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params: stream is required") << std::endl; - this->do_write(); - return; - } - auto stream = object["params"]["stream"].get(); - - // Validate stream name format - if (!isValidStreamName(stream)) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params: stream name invalid") << std::endl; - this->do_write(); - return; - } - - // Validate key param - if (!object["params"].contains("key") || !object["params"]["key"].is_string() - || object["params"]["key"].get().empty()) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params: key is required") << std::endl; - this->do_write(); - return; - } - auto key = object["params"]["key"].get(); - - // Get data (optional, defaults to empty) - std::string data; - if (object["params"].contains("data") && object["params"]["data"].is_string()) { - data = object["params"]["data"].get(); - } - - // Validate data size - static constexpr size_t kMaxDataSize = 128ULL * 1024 * 1024; - if (data.size() > kMaxDataSize) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params: data exceeds 128 MB limit") << std::endl; - this->do_write(); - return; - } - - // Per-node stream permissions - if (!allowed_streams.empty()) { - if (std::find(allowed_streams.begin(), allowed_streams.end(), stream) == allowed_streams.end()) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32003, "Stream not permitted on this node") << std::endl; - this->do_write(); - return; - } - } - - // Get optional keys for index - std::vector keys; - if (object["params"].contains("keys") && object["params"]["keys"].is_array()) { - keys = object["params"]["keys"].get>(); - } - - try { - Block b = bc.publish(stream, key, data, keys); - b.dump(); - bc.saveChunk(b.index / bc.chunkSize); - bc.saveKeys(); - - // Broadcast the new block to all connected peers - if (peer_manager) { - peer_manager->broadcast_block(b); - } - - buffer.consume(buffer.size()); - outputStream << resultMessage(object["id"], b.toJson().dump()) << std::endl; - } catch (const std::runtime_error &e) { - buffer.consume(buffer.size()); - outputStream << miningTimeoutMessage(object["id"], e.what()) << std::endl; - } - this->do_write(); - return; + std::string method = object["method"].get(); + auto it = dispatch_.find(method); + nlohmann::json response; + if (it != dispatch_.end()) { + response = it->second(object); + } else { + response = invalidMethodMessage(object["id"], method); } + buffer.consume(buffer.size()); + outputStream << response << std::endl; + this->do_write(); + } + }); +} - if(object["method"] == "createStream") - { - if (object["params"] == nullptr || !object["params"].contains("name") - || !object["params"]["name"].is_string() - || object["params"]["name"].get().empty()) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params: name is required") << std::endl; - this->do_write(); - return; - } - auto name = object["params"]["name"].get(); - if (!isValidStreamName(name)) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params: stream name invalid") << std::endl; - this->do_write(); - return; - } - try { - bc.createStream(name); - buffer.consume(buffer.size()); - outputStream << resultMessage(object["id"], "Stream '" + name + "' created") << std::endl; - } catch (const std::runtime_error &) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32004, "Stream already exists") << std::endl; - } - this->do_write(); - return; - } +void RpcServer::init_dispatch() +{ + dispatch_["publish"] = [this](const nlohmann::json &req) { return handle_publish(req); }; + dispatch_["createStream"] = [this](const nlohmann::json &req) { return handle_createStream(req); }; + dispatch_["listStreams"] = [this](const nlohmann::json &req) { return handle_listStreams(req); }; + dispatch_["getStreamEntries"] = [this](const nlohmann::json &req) { return handle_getStreamEntries(req); }; + dispatch_["getStreamEntry"] = [this](const nlohmann::json &req) { return handle_getStreamEntry(req); }; + dispatch_["requestSync"] = [this](const nlohmann::json &req) { return handle_requestSync(req); }; + dispatch_["getBlockByIndex"] = [this](const nlohmann::json &req) { return handle_getBlockByIndex(req); }; + dispatch_["getBlocksByKeys"] = [this](const nlohmann::json &req) { return handle_getBlocksByKeys(req); }; + dispatch_["addPeer"] = [this](const nlohmann::json &req) { return handle_addPeer(req); }; + dispatch_["removePeer"] = [this](const nlohmann::json &req) { return handle_removePeer(req); }; + dispatch_["listPeers"] = [this](const nlohmann::json &req) { return handle_listPeers(req); }; + dispatch_["banPeer"] = [this](const nlohmann::json &req) { return handle_banPeer(req); }; + dispatch_["unbanPeer"] = [this](const nlohmann::json &req) { return handle_unbanPeer(req); }; + dispatch_["getInclusionProof"] = [this](const nlohmann::json &req) { return handle_getInclusionProof(req); }; + dispatch_["verifyInclusionProof"] = [this](const nlohmann::json &req) { return handle_verifyInclusionProof(req); }; + dispatch_["getBlockHeader"] = [this](const nlohmann::json &req) { return handle_getBlockHeader(req); }; + dispatch_["getNodeStatus"] = [this](const nlohmann::json &req) { return handle_getNodeStatus(req); }; + dispatch_["getBlockRange"] = [this](const nlohmann::json &req) { return handle_getBlockRange(req); }; + dispatch_["getChainLength"] = [this](const nlohmann::json &req) { return handle_getChainLength(req); }; + dispatch_["getChunkCount"] = [this](const nlohmann::json &req) { return handle_getChunkCount(req); }; +} - if(object["method"] == "listStreams") - { - auto streams = bc.listStreams(); - nlohmann::json arr = nlohmann::json::array(); - for (const auto &s : streams) { - arr.push_back(s); - } - buffer.consume(buffer.size()); - outputStream << resultMessage(object["id"], arr.dump()) << std::endl; - this->do_write(); - return; - } +nlohmann::json RpcServer::handle_publish(const nlohmann::json &request) +{ + if (sync_status && sync_status->isSyncing.load()) { + return syncInProgressMessage(request["id"]); + } + + if (request["params"] == nullptr || request["params"].type() != nlohmann::json::value_t::object) { + return errorMessage(request["id"], -32602, "Invalid params"); + } + + if (!request["params"].contains("stream") || !request["params"]["stream"].is_string() + || request["params"]["stream"].get().empty()) { + return errorMessage(request["id"], -32602, "Invalid params: stream is required"); + } + auto stream_name = request["params"]["stream"].get(); + + if (!isValidStreamName(stream_name)) { + return errorMessage(request["id"], -32602, "Invalid params: stream name invalid"); + } + + if (!request["params"].contains("key") || !request["params"]["key"].is_string() + || request["params"]["key"].get().empty()) { + return errorMessage(request["id"], -32602, "Invalid params: key is required"); + } + auto key = request["params"]["key"].get(); + + std::string data; + if (request["params"].contains("data") && request["params"]["data"].is_string()) { + data = request["params"]["data"].get(); + } + + static constexpr size_t kMaxDataSize = 128ULL * 1024 * 1024; + if (data.size() > kMaxDataSize) { + return errorMessage(request["id"], -32602, "Invalid params: data exceeds 128 MB limit"); + } + + if (!allowed_streams.empty()) { + if (std::find(allowed_streams.begin(), allowed_streams.end(), stream_name) == allowed_streams.end()) { + return errorMessage(request["id"], -32003, "Stream not permitted on this node"); + } + } + + std::vector keys; + if (request["params"].contains("keys") && request["params"]["keys"].is_array()) { + keys = request["params"]["keys"].get>(); + } + + try { + Block b = bc.publish(stream_name, key, data, keys); + b.dump(); + bc.saveChunk(b.index / bc.chunkSize); + bc.saveKeys(); + + if (peer_manager) { + peer_manager->broadcast_block(b); + } + + return resultMessage(request["id"], b.toJson().dump()); + } catch (const std::runtime_error &e) { + return miningTimeoutMessage(request["id"], e.what()); + } +} - if(object["method"] == "getStreamEntries") - { - if (object["params"] == nullptr || !object["params"].contains("stream") - || !object["params"]["stream"].is_string() - || object["params"]["stream"].get().empty()) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params: stream is required") << std::endl; - this->do_write(); - return; - } - auto stream = object["params"]["stream"].get(); - std::string key; - if (object["params"].contains("key") && object["params"]["key"].is_string()) { - key = object["params"]["key"].get(); - } - auto entries = bc.getStreamEntries(stream, key); - nlohmann::json arr = nlohmann::json::array(); - for (const auto &[blockIdx, entry] : entries) { - nlohmann::json ej; - ej["block_index"] = blockIdx; - ej["stream"] = entry.stream; - ej["key"] = entry.key; - ej["data"] = entry.data; - arr.push_back(ej); - } - buffer.consume(buffer.size()); - outputStream << resultMessage(object["id"], arr.dump()) << std::endl; - this->do_write(); - return; - } +nlohmann::json RpcServer::handle_createStream(const nlohmann::json &request) +{ + if (request["params"] == nullptr || !request["params"].contains("name") + || !request["params"]["name"].is_string() + || request["params"]["name"].get().empty()) { + return errorMessage(request["id"], -32602, "Invalid params: name is required"); + } + auto name = request["params"]["name"].get(); + if (!isValidStreamName(name)) { + return errorMessage(request["id"], -32602, "Invalid params: stream name invalid"); + } + try { + bc.createStream(name); + return resultMessage(request["id"], "Stream '" + name + "' created"); + } catch (const std::runtime_error &) { + return errorMessage(request["id"], -32004, "Stream already exists"); + } +} - if(object["method"] == "getStreamEntry") - { - if (object["params"] == nullptr - || !object["params"].contains("stream") || !object["params"]["stream"].is_string() - || object["params"]["stream"].get().empty() - || !object["params"].contains("key") || !object["params"]["key"].is_string() - || object["params"]["key"].get().empty()) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params: stream and key are required") << std::endl; - this->do_write(); - return; - } - auto stream = object["params"]["stream"].get(); - auto key = object["params"]["key"].get(); - try { - auto [blockIdx, entry] = bc.getStreamEntry(stream, key); - nlohmann::json ej; - ej["block_index"] = blockIdx; - ej["stream"] = entry.stream; - ej["key"] = entry.key; - ej["data"] = entry.data; - buffer.consume(buffer.size()); - outputStream << resultMessage(object["id"], ej.dump()) << std::endl; - } catch (const std::runtime_error &) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32601, "Entry not found") << std::endl; - } - this->do_write(); - return; - } +nlohmann::json RpcServer::handle_listStreams(const nlohmann::json &request) +{ + auto streams = bc.listStreams(); + nlohmann::json arr = nlohmann::json::array(); + for (const auto &s : streams) { + arr.push_back(s); + } + return resultMessage(request["id"], arr.dump()); +} - if(object["method"] == "requestSync") - { - if (sync_status && sync_status->isSyncing.load()) { - buffer.consume(buffer.size()); - outputStream << syncAlreadyInProgressMessage(object["id"]) << std::endl; - this->do_write(); - return; - } - - if (!peer_client || !peer_client->is_connected()) { - buffer.consume(buffer.size()); - outputStream << noPeerMessage(object["id"]) << std::endl; - this->do_write(); - return; - } - - peer_client->start_sync(); - buffer.consume(buffer.size()); - outputStream << syncStartedMessage(object["id"]) << std::endl; - this->do_write(); - return; - } +nlohmann::json RpcServer::handle_getStreamEntries(const nlohmann::json &request) +{ + if (request["params"] == nullptr || !request["params"].contains("stream") + || !request["params"]["stream"].is_string() + || request["params"]["stream"].get().empty()) { + return errorMessage(request["id"], -32602, "Invalid params: stream is required"); + } + auto stream_name = request["params"]["stream"].get(); + std::string key; + if (request["params"].contains("key") && request["params"]["key"].is_string()) { + key = request["params"]["key"].get(); + } + auto entries = bc.getStreamEntries(stream_name, key); + nlohmann::json arr = nlohmann::json::array(); + for (const auto &[blockIdx, entry] : entries) { + nlohmann::json ej; + ej["block_index"] = blockIdx; + ej["stream"] = entry.stream; + ej["key"] = entry.key; + ej["data"] = entry.data; + arr.push_back(ej); + } + return resultMessage(request["id"], arr.dump()); +} - if(object["method"] == "getBlockByIndex") - { - if(object["params"] == nullptr || object["params"].type() != nlohmann::json::value_t::object || object["params"]["index"] == nullptr) - { - buffer.consume(buffer.size()); - outputStream << invalidParamsMessage(object["id"]) << std::endl; - this->do_write(); - return; - } - auto index = object["params"]["index"].get(); - if (index >= bc.getChainLength()) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32001, "Block not found") << std::endl; - this->do_write(); - return; - } - Block b = bc.getBlockByIndex(index); - b.dump(); - buffer.consume(buffer.size()); - outputStream << resultMessage(object["id"], b.toJson().dump()) << std::endl; - this->do_write(); - return; - } +nlohmann::json RpcServer::handle_getStreamEntry(const nlohmann::json &request) +{ + if (request["params"] == nullptr + || !request["params"].contains("stream") || !request["params"]["stream"].is_string() + || request["params"]["stream"].get().empty() + || !request["params"].contains("key") || !request["params"]["key"].is_string() + || request["params"]["key"].get().empty()) { + return errorMessage(request["id"], -32602, "Invalid params: stream and key are required"); + } + auto stream_name = request["params"]["stream"].get(); + auto key = request["params"]["key"].get(); + try { + auto [blockIdx, entry] = bc.getStreamEntry(stream_name, key); + nlohmann::json ej; + ej["block_index"] = blockIdx; + ej["stream"] = entry.stream; + ej["key"] = entry.key; + ej["data"] = entry.data; + return resultMessage(request["id"], ej.dump()); + } catch (const std::runtime_error &) { + return errorMessage(request["id"], -32601, "Entry not found"); + } +} - if(object["method"] == "getBlocksByKeys") - { - if(object["params"] == nullptr || object["params"]["keys"] == nullptr) - { - buffer.consume(buffer.size()); - outputStream << invalidParamsMessage(object["id"]) << std::endl; - this->do_write(); - return; - } - auto keys = object["params"]["keys"].get>(); - std::vector blocks = bc.getBlocksByKeys(keys); - nlohmann::json response; - - for(auto &b : blocks) - { - response.push_back(b.toJson()); - } - - buffer.consume(buffer.size()); - outputStream << resultMessage(object["id"], response.dump()) << std::endl; - this->do_write(); - return; - } +nlohmann::json RpcServer::handle_requestSync(const nlohmann::json &request) +{ + if (sync_status && sync_status->isSyncing.load()) { + return syncAlreadyInProgressMessage(request["id"]); + } - if(object["method"] == "addPeer") - { - if (!peer_manager) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32603, "Peer manager not available") << std::endl; - this->do_write(); - return; - } - if (object["params"] == nullptr || !object["params"].contains("host") || !object["params"].contains("port")) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params: host and port are required") << std::endl; - this->do_write(); - return; - } - auto host = object["params"]["host"].get(); - auto port = object["params"]["port"].get(); - - if (peer_manager->is_banned(host, port)) { - auto bans = peer_manager->get_bans(); - for (const auto &ban : bans) { - if (ban.host == host && ban.port == port) { - buffer.consume(buffer.size()); - outputStream << errorMessageWithData(object["id"], -32004, "Peer is currently banned", {{"expires", ban.expires}}) << std::endl; - this->do_write(); - return; - } - } - } - - if (peer_manager->outbound_count() >= peer_manager->get_config().max_outbound) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32003, "Outbound connection limit reached") << std::endl; - this->do_write(); - return; - } - - peer_manager->connect_to(host, port); - PeerEntry entry; - entry.host = host; - entry.port = port; - entry.last_seen = static_cast(std::time(nullptr)); - peer_manager->add_peer(entry); - peer_manager->save_peers(); - buffer.consume(buffer.size()); - outputStream << resultMessage(object["id"], "peer_added") << std::endl; - this->do_write(); - return; - } + if (!peer_client || !peer_client->is_connected()) { + return noPeerMessage(request["id"]); + } - if(object["method"] == "removePeer") - { - if (!peer_manager) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32603, "Peer manager not available") << std::endl; - this->do_write(); - return; - } - if (object["params"] == nullptr || !object["params"].contains("host") || !object["params"].contains("port")) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params: host and port are required") << std::endl; - this->do_write(); - return; - } - auto host = object["params"]["host"].get(); - auto port = object["params"]["port"].get(); - - if (!peer_manager->find_peer(host, port)) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32005, "Peer not found") << std::endl; - this->do_write(); - return; - } - - peer_manager->disconnect_and_remove(host, port); - buffer.consume(buffer.size()); - outputStream << resultMessage(object["id"], "peer_removed") << std::endl; - this->do_write(); - return; - } + peer_client->start_sync(); + return syncStartedMessage(request["id"]); +} - if(object["method"] == "listPeers") - { - if (!peer_manager) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32603, "Peer manager not available") << std::endl; - this->do_write(); - return; - } - - nlohmann::json result; - result["node_uuid"] = peer_manager->get_node_uuid(); - result["discovery_enabled"] = peer_manager->is_discovery_enabled(); - result["outbound_count"] = peer_manager->outbound_count(); - result["inbound_count"] = peer_manager->inbound_count(); - result["max_outbound"] = peer_manager->get_config().max_outbound; - result["max_inbound"] = peer_manager->get_config().max_inbound; - - nlohmann::json peers_json = nlohmann::json::array(); - for (const auto &p : peer_manager->get_peers()) { - nlohmann::json pj; - pj["host"] = p.host; - pj["port"] = p.port; - pj["node_uuid"] = p.node_uuid; - pj["last_seen"] = p.last_seen; - pj["error_count"] = p.error_count; - peers_json.push_back(pj); - } - result["peers"] = peers_json; - - nlohmann::json bans_json = nlohmann::json::array(); - for (const auto &b : peer_manager->get_bans()) { - nlohmann::json bj; - bj["host"] = b.host; - bj["port"] = b.port; - bj["reason"] = b.reason; - bj["expires"] = b.expires; - bans_json.push_back(bj); - } - result["bans"] = bans_json; +nlohmann::json RpcServer::handle_getBlockByIndex(const nlohmann::json &request) +{ + if (request["params"] == nullptr || request["params"].type() != nlohmann::json::value_t::object || request["params"]["index"] == nullptr) { + return invalidParamsMessage(request["id"]); + } + auto index = request["params"]["index"].get(); + if (index >= bc.getChainLength()) { + return errorMessage(request["id"], -32001, "Block not found"); + } + Block b = bc.getBlockByIndex(index); + b.dump(); + return resultMessage(request["id"], b.toJson().dump()); +} - buffer.consume(buffer.size()); - outputStream << resultJsonMessage(object["id"], result) << std::endl; - this->do_write(); - return; - } +nlohmann::json RpcServer::handle_getBlocksByKeys(const nlohmann::json &request) +{ + if (request["params"] == nullptr || request["params"]["keys"] == nullptr) { + return invalidParamsMessage(request["id"]); + } + auto keys = request["params"]["keys"].get>(); + std::vector blocks = bc.getBlocksByKeys(keys); + nlohmann::json result; + + for (auto &b : blocks) { + result.push_back(b.toJson()); + } + + return resultMessage(request["id"], result.dump()); +} - if(object["method"] == "banPeer") - { - if (!peer_manager) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32603, "Peer manager not available") << std::endl; - this->do_write(); - return; - } - if (object["params"] == nullptr || !object["params"].contains("host") || !object["params"].contains("port")) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params: host and port are required") << std::endl; - this->do_write(); - return; - } - auto host = object["params"]["host"].get(); - auto port = object["params"]["port"].get(); - uint64_t duration = peer_manager->get_config().ban_duration_seconds; - if (object["params"].contains("duration_seconds")) { - duration = object["params"]["duration_seconds"].get(); - } - - peer_manager->ban_peer(host, port, "manual", duration); - buffer.consume(buffer.size()); - outputStream << resultMessage(object["id"], "peer_banned") << std::endl; - this->do_write(); - return; - } +nlohmann::json RpcServer::handle_addPeer(const nlohmann::json &request) +{ + if (!peer_manager) { + return errorMessage(request["id"], -32603, "Peer manager not available"); + } + if (request["params"] == nullptr || !request["params"].contains("host") || !request["params"].contains("port")) { + return errorMessage(request["id"], -32602, "Invalid params: host and port are required"); + } + auto host = request["params"]["host"].get(); + auto port = request["params"]["port"].get(); + + if (peer_manager->is_banned(host, port)) { + auto bans = peer_manager->get_bans(); + for (const auto &ban : bans) { + if (ban.host == host && ban.port == port) { + return errorMessageWithData(request["id"], -32004, "Peer is currently banned", {{"expires", ban.expires}}); + } + } + } + + if (peer_manager->outbound_count() >= peer_manager->get_config().max_outbound) { + return errorMessage(request["id"], -32003, "Outbound connection limit reached"); + } + + peer_manager->connect_to(host, port); + PeerEntry entry; + entry.host = host; + entry.port = port; + entry.last_seen = static_cast(std::time(nullptr)); + peer_manager->add_peer(entry); + peer_manager->save_peers(); + return resultMessage(request["id"], "peer_added"); +} - if(object["method"] == "unbanPeer") - { - if (!peer_manager) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32603, "Peer manager not available") << std::endl; - this->do_write(); - return; - } - if (object["params"] == nullptr || !object["params"].contains("host") || !object["params"].contains("port")) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params: host and port are required") << std::endl; - this->do_write(); - return; - } - auto host = object["params"]["host"].get(); - auto port = object["params"]["port"].get(); - - if (!peer_manager->is_banned(host, port)) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32006, "Peer is not banned") << std::endl; - this->do_write(); - return; - } - - peer_manager->unban_peer(host, port); - peer_manager->save_peers(); - buffer.consume(buffer.size()); - outputStream << resultMessage(object["id"], "peer_unbanned") << std::endl; - this->do_write(); - return; - } +nlohmann::json RpcServer::handle_removePeer(const nlohmann::json &request) +{ + if (!peer_manager) { + return errorMessage(request["id"], -32603, "Peer manager not available"); + } + if (request["params"] == nullptr || !request["params"].contains("host") || !request["params"].contains("port")) { + return errorMessage(request["id"], -32602, "Invalid params: host and port are required"); + } + auto host = request["params"]["host"].get(); + auto port = request["params"]["port"].get(); + + if (!peer_manager->find_peer(host, port)) { + return errorMessage(request["id"], -32005, "Peer not found"); + } + + peer_manager->disconnect_and_remove(host, port); + return resultMessage(request["id"], "peer_removed"); +} - if(object["method"] == "getInclusionProof") - { - if (object["params"] == nullptr || object["params"].type() != nlohmann::json::value_t::object - || !object["params"].contains("blockIndex") || !object["params"]["blockIndex"].is_number_integer() - || !object["params"].contains("entryIndex") || !object["params"]["entryIndex"].is_number_integer()) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params") << std::endl; - this->do_write(); - return; - } - auto blockIndex = object["params"]["blockIndex"].get(); - auto entryIndex = object["params"]["entryIndex"].get(); - try { - auto result = bc.getInclusionProof(blockIndex, entryIndex); - buffer.consume(buffer.size()); - outputStream << resultJsonMessage(object["id"], result) << std::endl; - } catch (const std::out_of_range &e) { - std::string msg = e.what(); - int code = -32001; - if (msg.find("Entry") != std::string::npos) { - code = -32002; - msg = "Entry not found"; - } else { - msg = "Block not found"; - } - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], code, msg) << std::endl; - } - this->do_write(); - return; - } +nlohmann::json RpcServer::handle_listPeers(const nlohmann::json &request) +{ + if (!peer_manager) { + return errorMessage(request["id"], -32603, "Peer manager not available"); + } + + nlohmann::json result; + result["node_uuid"] = peer_manager->get_node_uuid(); + result["discovery_enabled"] = peer_manager->is_discovery_enabled(); + result["outbound_count"] = peer_manager->outbound_count(); + result["inbound_count"] = peer_manager->inbound_count(); + result["max_outbound"] = peer_manager->get_config().max_outbound; + result["max_inbound"] = peer_manager->get_config().max_inbound; + + nlohmann::json peers_json = nlohmann::json::array(); + for (const auto &p : peer_manager->get_peers()) { + nlohmann::json pj; + pj["host"] = p.host; + pj["port"] = p.port; + pj["node_uuid"] = p.node_uuid; + pj["last_seen"] = p.last_seen; + pj["error_count"] = p.error_count; + peers_json.push_back(pj); + } + result["peers"] = peers_json; + + nlohmann::json bans_json = nlohmann::json::array(); + for (const auto &b : peer_manager->get_bans()) { + nlohmann::json bj; + bj["host"] = b.host; + bj["port"] = b.port; + bj["reason"] = b.reason; + bj["expires"] = b.expires; + bans_json.push_back(bj); + } + result["bans"] = bans_json; + + return resultJsonMessage(request["id"], result); +} - if(object["method"] == "verifyInclusionProof") - { - if (object["params"] == nullptr || object["params"].type() != nlohmann::json::value_t::object - || !object["params"].contains("blockIndex") || !object["params"]["blockIndex"].is_number_integer() - || !object["params"].contains("leafHash") || !object["params"]["leafHash"].is_string() - || !object["params"].contains("proof") || !object["params"]["proof"].is_array()) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params") << std::endl; - this->do_write(); - return; - } - // Validate proof array elements - for (const auto &elem : object["params"]["proof"]) { - if (!elem.contains("hash") || !elem["hash"].is_string() - || !elem.contains("isLeft") || !elem["isLeft"].is_boolean()) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params") << std::endl; - this->do_write(); - return; - } - } - auto blockIndex = object["params"]["blockIndex"].get(); - auto leafHash = object["params"]["leafHash"].get(); - auto proofArray = object["params"]["proof"]; - try { - auto result = bc.verifyInclusionProof(blockIndex, leafHash, proofArray); - buffer.consume(buffer.size()); - outputStream << resultJsonMessage(object["id"], result) << std::endl; - } catch (const std::out_of_range &) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32001, "Block not found") << std::endl; - } - this->do_write(); - return; - } +nlohmann::json RpcServer::handle_banPeer(const nlohmann::json &request) +{ + if (!peer_manager) { + return errorMessage(request["id"], -32603, "Peer manager not available"); + } + if (request["params"] == nullptr || !request["params"].contains("host") || !request["params"].contains("port")) { + return errorMessage(request["id"], -32602, "Invalid params: host and port are required"); + } + auto host = request["params"]["host"].get(); + auto port = request["params"]["port"].get(); + uint64_t duration = peer_manager->get_config().ban_duration_seconds; + if (request["params"].contains("duration_seconds")) { + duration = request["params"]["duration_seconds"].get(); + } + + peer_manager->ban_peer(host, port, "manual", duration); + return resultMessage(request["id"], "peer_banned"); +} - if(object["method"] == "getBlockHeader") - { - if (object["params"] == nullptr || object["params"].type() != nlohmann::json::value_t::object - || !object["params"].contains("blockIndex") || !object["params"]["blockIndex"].is_number_integer()) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params") << std::endl; - this->do_write(); - return; - } - auto blockIndex = object["params"]["blockIndex"].get(); - try { - Block b = bc.getBlockByIndex(blockIndex); - buffer.consume(buffer.size()); - outputStream << resultJsonMessage(object["id"], b.toHeaderJson()) << std::endl; - } catch (const std::out_of_range &) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32001, "Block not found") << std::endl; - } - this->do_write(); - return; - } - - if(object["method"] == "getNodeStatus") - { - nlohmann::json result; - result["chainLength"] = bc.getChainLength(); - result["chunkCount"] = bc.getChunkCount(); - result["syncState"] = (sync_status && sync_status->isSyncing.load()) ? "syncing" : "idle"; - result["currentDifficulty"] = bc.getCurrentDifficulty(); - result["inboundPeers"] = peer_manager ? peer_manager->inbound_count() : static_cast(0); - result["outboundPeers"] = peer_manager ? peer_manager->outbound_count() : static_cast(0); - result["nodeUuid"] = peer_manager ? peer_manager->get_node_uuid() : ""; - buffer.consume(buffer.size()); - outputStream << resultJsonMessage(object["id"], result) << std::endl; - this->do_write(); - return; - } +nlohmann::json RpcServer::handle_unbanPeer(const nlohmann::json &request) +{ + if (!peer_manager) { + return errorMessage(request["id"], -32603, "Peer manager not available"); + } + if (request["params"] == nullptr || !request["params"].contains("host") || !request["params"].contains("port")) { + return errorMessage(request["id"], -32602, "Invalid params: host and port are required"); + } + auto host = request["params"]["host"].get(); + auto port = request["params"]["port"].get(); + + if (!peer_manager->is_banned(host, port)) { + return errorMessage(request["id"], -32006, "Peer is not banned"); + } + + peer_manager->unban_peer(host, port); + peer_manager->save_peers(); + return resultMessage(request["id"], "peer_unbanned"); +} - if(object["method"] == "getBlockRange") - { - if (object["params"] == nullptr || object["params"].type() != nlohmann::json::value_t::object - || !object["params"].contains("startIndex") || !object["params"]["startIndex"].is_number_integer() - || !object["params"].contains("endIndex") || !object["params"]["endIndex"].is_number_integer()) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid params") << std::endl; - this->do_write(); - return; - } - auto startIndex = object["params"]["startIndex"].get(); - auto endIndex = object["params"]["endIndex"].get(); - bool headersOnly = false; - if (object["params"].contains("headersOnly") && object["params"]["headersOnly"].is_boolean()) { - headersOnly = object["params"]["headersOnly"].get(); - } - - if (startIndex > endIndex) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Invalid range: startIndex exceeds endIndex") << std::endl; - this->do_write(); - return; - } - - static constexpr size_t kMaxBlockRange = 1000; - if (endIndex - startIndex + 1 > kMaxBlockRange) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32602, "Range too large: maximum 1000 blocks per request") << std::endl; - this->do_write(); - return; - } - - size_t chainLength = bc.getChainLength(); - if (startIndex >= chainLength) { - buffer.consume(buffer.size()); - outputStream << errorMessage(object["id"], -32001, "Start index out of range") << std::endl; - this->do_write(); - return; - } - - if (endIndex >= chainLength) { - endIndex = chainLength - 1; - } - - nlohmann::json blocks = nlohmann::json::array(); - for (size_t i = startIndex; i <= endIndex; i++) { - Block b = bc.getBlockByIndex(i); - blocks.push_back(headersOnly ? b.toHeaderJson() : b.toJson()); - } - buffer.consume(buffer.size()); - outputStream << resultMessage(object["id"], blocks.dump()) << std::endl; - this->do_write(); - return; - } +nlohmann::json RpcServer::handle_getInclusionProof(const nlohmann::json &request) +{ + if (request["params"] == nullptr || request["params"].type() != nlohmann::json::value_t::object + || !request["params"].contains("blockIndex") || !request["params"]["blockIndex"].is_number_integer() + || !request["params"].contains("entryIndex") || !request["params"]["entryIndex"].is_number_integer()) { + return errorMessage(request["id"], -32602, "Invalid params"); + } + auto blockIndex = request["params"]["blockIndex"].get(); + auto entryIndex = request["params"]["entryIndex"].get(); + try { + auto result = bc.getInclusionProof(blockIndex, entryIndex); + return resultJsonMessage(request["id"], result); + } catch (const std::out_of_range &e) { + std::string msg = e.what(); + int code = -32001; + if (msg.find("Entry") != std::string::npos) { + code = -32002; + msg = "Entry not found"; + } else { + msg = "Block not found"; + } + return errorMessage(request["id"], code, msg); + } +} - if(object["method"] == "getChainLength") - { - buffer.consume(buffer.size()); - outputStream << resultMessage(object["id"], std::to_string(bc.getChainLength())) << std::endl; - this->do_write(); - return; - } +nlohmann::json RpcServer::handle_verifyInclusionProof(const nlohmann::json &request) +{ + if (request["params"] == nullptr || request["params"].type() != nlohmann::json::value_t::object + || !request["params"].contains("blockIndex") || !request["params"]["blockIndex"].is_number_integer() + || !request["params"].contains("leafHash") || !request["params"]["leafHash"].is_string() + || !request["params"].contains("proof") || !request["params"]["proof"].is_array()) { + return errorMessage(request["id"], -32602, "Invalid params"); + } + for (const auto &elem : request["params"]["proof"]) { + if (!elem.contains("hash") || !elem["hash"].is_string() + || !elem.contains("isLeft") || !elem["isLeft"].is_boolean()) { + return errorMessage(request["id"], -32602, "Invalid params"); + } + } + auto blockIndex = request["params"]["blockIndex"].get(); + auto leafHash = request["params"]["leafHash"].get(); + auto proofArray = request["params"]["proof"]; + try { + auto result = bc.verifyInclusionProof(blockIndex, leafHash, proofArray); + return resultJsonMessage(request["id"], result); + } catch (const std::out_of_range &) { + return errorMessage(request["id"], -32001, "Block not found"); + } +} - if(object["method"] == "getChunkCount") - { - buffer.consume(buffer.size()); - outputStream << resultMessage(object["id"], std::to_string(bc.getChunkCount())) << std::endl; - this->do_write(); - return; - } +nlohmann::json RpcServer::handle_getBlockHeader(const nlohmann::json &request) +{ + if (request["params"] == nullptr || request["params"].type() != nlohmann::json::value_t::object + || !request["params"].contains("blockIndex") || !request["params"]["blockIndex"].is_number_integer()) { + return errorMessage(request["id"], -32602, "Invalid params"); + } + auto blockIndex = request["params"]["blockIndex"].get(); + try { + Block b = bc.getBlockByIndex(blockIndex); + return resultJsonMessage(request["id"], b.toHeaderJson()); + } catch (const std::out_of_range &) { + return errorMessage(request["id"], -32001, "Block not found"); + } +} - buffer.consume(buffer.size()); - outputStream << invalidMethodMessage(object["id"], object["method"]) << std::endl; - this->do_write(); - } - }); +nlohmann::json RpcServer::handle_getNodeStatus(const nlohmann::json &request) +{ + nlohmann::json result; + result["chainLength"] = bc.getChainLength(); + result["chunkCount"] = bc.getChunkCount(); + result["syncState"] = (sync_status && sync_status->isSyncing.load()) ? "syncing" : "idle"; + result["currentDifficulty"] = bc.getCurrentDifficulty(); + result["inboundPeers"] = peer_manager ? peer_manager->inbound_count() : static_cast(0); + result["outboundPeers"] = peer_manager ? peer_manager->outbound_count() : static_cast(0); + result["nodeUuid"] = peer_manager ? peer_manager->get_node_uuid() : ""; + return resultJsonMessage(request["id"], result); +} + +nlohmann::json RpcServer::handle_getBlockRange(const nlohmann::json &request) +{ + if (request["params"] == nullptr || request["params"].type() != nlohmann::json::value_t::object + || !request["params"].contains("startIndex") || !request["params"]["startIndex"].is_number_integer() + || !request["params"].contains("endIndex") || !request["params"]["endIndex"].is_number_integer()) { + return errorMessage(request["id"], -32602, "Invalid params"); + } + auto startIndex = request["params"]["startIndex"].get(); + auto endIndex = request["params"]["endIndex"].get(); + bool headersOnly = false; + if (request["params"].contains("headersOnly") && request["params"]["headersOnly"].is_boolean()) { + headersOnly = request["params"]["headersOnly"].get(); + } + + if (startIndex > endIndex) { + return errorMessage(request["id"], -32602, "Invalid range: startIndex exceeds endIndex"); + } + + static constexpr size_t kMaxBlockRange = 1000; + if (endIndex - startIndex + 1 > kMaxBlockRange) { + return errorMessage(request["id"], -32602, "Range too large: maximum 1000 blocks per request"); + } + + size_t chainLength = bc.getChainLength(); + if (startIndex >= chainLength) { + return errorMessage(request["id"], -32001, "Start index out of range"); + } + + if (endIndex >= chainLength) { + endIndex = chainLength - 1; + } + + nlohmann::json blocks = nlohmann::json::array(); + for (size_t i = startIndex; i <= endIndex; i++) { + Block b = bc.getBlockByIndex(i); + blocks.push_back(headersOnly ? b.toHeaderJson() : b.toJson()); + } + return resultMessage(request["id"], blocks.dump()); +} + +nlohmann::json RpcServer::handle_getChainLength(const nlohmann::json &request) +{ + return resultMessage(request["id"], std::to_string(bc.getChainLength())); +} + +nlohmann::json RpcServer::handle_getChunkCount(const nlohmann::json &request) +{ + return resultMessage(request["id"], std::to_string(bc.getChunkCount())); } void RpcServer::do_write() diff --git a/src/network/RpcServer.hpp b/src/network/RpcServer.hpp index d21a406..53e019d 100644 --- a/src/network/RpcServer.hpp +++ b/src/network/RpcServer.hpp @@ -7,6 +7,8 @@ #include #include #include +#include +#include #include "../IBlockchain.hpp" #include "../Chunk.hpp" #include "../json.hpp" @@ -21,12 +23,16 @@ class PeerManager; class RpcServer : public SessionHandler, public std::enable_shared_from_this { + public: + using RpcHandler = std::function; + private: boost::asio::streambuf buffer; SyncStatus *sync_status = nullptr; PeerClient *peer_client = nullptr; PeerManager *peer_manager = nullptr; std::vector allowed_streams; + std::unordered_map dispatch_; protected: std::shared_ptr shared_self() override { return shared_from_this(); } @@ -44,6 +50,28 @@ class RpcServer : public SessionHandler, public std::enable_shared_from_this parsePeerKey(const std::string &key); void logMessage(const std::string &level, const std::string &msg); bool checkLeadingZeroBits(const std::string &hashStr, uint32_t bitsNeeded); -std::string generate_uuid_v4(); \ No newline at end of file +std::string generate_uuid_v4(); + +// Lazy log macros — suppress message expression evaluation when the level is below threshold +#define LOG_DEBUG(msg) do { if (getLogLevel() <= LogLevel::Debug) logMessage("DEBUG", msg); } while (0) +#define LOG_INFO(msg) do { if (getLogLevel() <= LogLevel::Info) logMessage("INFO", msg); } while (0) +#define LOG_WARN(msg) do { if (getLogLevel() <= LogLevel::Warning) logMessage("WARN", msg); } while (0) +#define LOG_ERROR(msg) do { if (getLogLevel() <= LogLevel::Error) logMessage("ERROR", msg); } while (0) \ No newline at end of file diff --git a/tests/TestHelpers.hpp b/tests/TestHelpers.hpp index c351931..643683c 100644 --- a/tests/TestHelpers.hpp +++ b/tests/TestHelpers.hpp @@ -74,4 +74,22 @@ inline std::vector buildValidChain(size_t length, uint32_t difficulty = 0 return chain; } +// Fast difficulty-0 block creation for chunk persistence tests +inline Block make_block(size_t index, const std::string &prevHash) { + StreamEntry e; + e.stream = "test"; + e.key = "k" + std::to_string(index); + e.data = "data"; + + Block b; + b.index = index; + b.timestamp = static_cast(std::time(nullptr)); + b.entries = {e}; + b.prevHash = prevHash; + b.difficulty = 0; + b.nonce = 0; + b.hash = b.calculateHash(); + return b; +} + } // namespace TestHelpers diff --git a/tests/chunk_persistence_tests.cpp b/tests/chunk_persistence_tests.cpp index 47df678..60c14d9 100644 --- a/tests/chunk_persistence_tests.cpp +++ b/tests/chunk_persistence_tests.cpp @@ -9,44 +9,10 @@ #include #include -namespace { - -// Helper: create a temp directory for test data -std::filesystem::path create_test_dir(const std::string &name) { - auto dir = std::filesystem::temp_directory_path() / ("chunk_persist_test_" + name); - std::filesystem::create_directories(dir); - return dir; -} - -// Helper: clean up test directory -void cleanup_test_dir(const std::filesystem::path &dir) { - std::filesystem::remove_all(dir); -} - -// Helper: build a valid block for a given index (difficulty 0 for speed) -Block make_block(size_t index, const std::string &prevHash) { - StreamEntry e; - e.stream = "test"; - e.key = "k" + std::to_string(index); - e.data = "data"; - - Block b; - b.index = index; - b.timestamp = static_cast(std::time(nullptr)); - b.entries = {e}; - b.prevHash = prevHash; - b.difficulty = 0; - b.nonce = 0; - b.hash = b.calculateHash(); - return b; -} - -} // anonymous namespace - // T011: chunk auto-saved when it reaches capacity (100 blocks) TEST_CASE("Chunk auto-saved when it reaches capacity", "[US1][persistence]") { - auto dir = create_test_dir("T011"); + auto dir = TestHelpers::createTestDir("T011"); { auto cfg = TestHelpers::defaultConsensusConfig(); Blockchain bc(dir, cfg); @@ -62,13 +28,13 @@ TEST_CASE("Chunk auto-saved when it reaches capacity", "[US1][persistence]") // chunk_000000.dat should now exist on disk REQUIRE(std::filesystem::exists(dir / "chunk_000000.dat")); } - cleanup_test_dir(dir); + TestHelpers::cleanupTestDir(dir); } // T012: filled chunk freed from memory after auto-save TEST_CASE("Filled chunk freed from memory after auto-save", "[US1][persistence]") { - auto dir = create_test_dir("T012"); + auto dir = TestHelpers::createTestDir("T012"); { auto cfg = TestHelpers::defaultConsensusConfig(); Blockchain bc(dir, cfg); @@ -87,13 +53,13 @@ TEST_CASE("Filled chunk freed from memory after auto-save", "[US1][persistence]" Block b = bc.getBlockByIndex(100); REQUIRE(b.index == 100); } - cleanup_test_dir(dir); + TestHelpers::cleanupTestDir(dir); } // T013: all in-memory chunks saved on shutdown call TEST_CASE("All in-memory chunks saved on shutdown call", "[US1][persistence]") { - auto dir = create_test_dir("T013"); + auto dir = TestHelpers::createTestDir("T013"); { auto cfg = TestHelpers::defaultConsensusConfig(); Blockchain bc(dir, cfg); @@ -112,13 +78,13 @@ TEST_CASE("All in-memory chunks saved on shutdown call", "[US1][persistence]") REQUIRE(std::filesystem::exists(dir / "streams.dat")); REQUIRE(std::filesystem::exists(dir / "stream_index.dat")); } - cleanup_test_dir(dir); + TestHelpers::cleanupTestDir(dir); } // T014: save failure logs error and continues operation TEST_CASE("Save failure logs error and continues operation", "[US1][persistence]") { - auto dir = create_test_dir("T014"); + auto dir = TestHelpers::createTestDir("T014"); { auto cfg = TestHelpers::defaultConsensusConfig(); Blockchain bc(dir, cfg); @@ -136,13 +102,13 @@ TEST_CASE("Save failure logs error and continues operation", "[US1][persistence] Block b = bc.getBlockByIndex(6); REQUIRE(b.index == 6); } - cleanup_test_dir(dir); + TestHelpers::cleanupTestDir(dir); } // T031: periodic timer fires and saves dirty active chunk TEST_CASE("Periodic timer fires and saves dirty active chunk", "[US3][persistence]") { - auto dir = create_test_dir("T031"); + auto dir = TestHelpers::createTestDir("T031"); { auto cfg = TestHelpers::defaultConsensusConfig(); Blockchain bc(dir, cfg); @@ -158,13 +124,13 @@ TEST_CASE("Periodic timer fires and saves dirty active chunk", "[US3][persistenc bc.stopPeriodicSave(); } - cleanup_test_dir(dir); + TestHelpers::cleanupTestDir(dir); } // T032: periodic timer skips save when dirty_ == false TEST_CASE("Periodic timer skips save when not dirty", "[US3][persistence]") { - auto dir = create_test_dir("T032"); + auto dir = TestHelpers::createTestDir("T032"); { auto cfg = TestHelpers::defaultConsensusConfig(); Blockchain bc(dir, cfg); @@ -179,13 +145,13 @@ TEST_CASE("Periodic timer skips save when not dirty", "[US3][persistence]") io.run_for(std::chrono::milliseconds(100)); bc.stopPeriodicSave(); } - cleanup_test_dir(dir); + TestHelpers::cleanupTestDir(dir); } // T033: periodic save disabled when save_interval_seconds == 0 TEST_CASE("Periodic save disabled when interval is 0", "[US3][persistence]") { - auto dir = create_test_dir("T033"); + auto dir = TestHelpers::createTestDir("T033"); { auto cfg = TestHelpers::defaultConsensusConfig(); Blockchain bc(dir, cfg); @@ -196,13 +162,13 @@ TEST_CASE("Periodic save disabled when interval is 0", "[US3][persistence]") io.run_for(std::chrono::milliseconds(100)); bc.stopPeriodicSave(); } - cleanup_test_dir(dir); + TestHelpers::cleanupTestDir(dir); } // T034: periodic save also saves index files TEST_CASE("Periodic save also saves index files", "[US3][persistence]") { - auto dir = create_test_dir("T034"); + auto dir = TestHelpers::createTestDir("T034"); { auto cfg = TestHelpers::defaultConsensusConfig(); Blockchain bc(dir, cfg); @@ -216,13 +182,13 @@ TEST_CASE("Periodic save also saves index files", "[US3][persistence]") REQUIRE(std::filesystem::exists(dir / "streams.dat")); REQUIRE(std::filesystem::exists(dir / "stream_index.dat")); } - cleanup_test_dir(dir); + TestHelpers::cleanupTestDir(dir); } // T046: getChainLength returns correct total across multiple chunks TEST_CASE("getChainLength returns correct total across multiple chunks", "[US5][persistence]") { - auto dir = create_test_dir("T046"); + auto dir = TestHelpers::createTestDir("T046"); { auto cfg = TestHelpers::defaultConsensusConfig(); Blockchain bc(dir, cfg); @@ -237,13 +203,13 @@ TEST_CASE("getChainLength returns correct total across multiple chunks", "[US5][ REQUIRE(bc.getChainLength() == 101); } - cleanup_test_dir(dir); + TestHelpers::cleanupTestDir(dir); } // T047: getChunkCount returns correct count TEST_CASE("getChunkCount returns correct count", "[US5][persistence]") { - auto dir = create_test_dir("T047"); + auto dir = TestHelpers::createTestDir("T047"); { auto cfg = TestHelpers::defaultConsensusConfig(); Blockchain bc(dir, cfg); @@ -262,13 +228,13 @@ TEST_CASE("getChunkCount returns correct count", "[US5][persistence]") bc.publish("test", "k100", "data", {"k100"}); REQUIRE(bc.getChunkCount() == 2); } - cleanup_test_dir(dir); + TestHelpers::cleanupTestDir(dir); } // T049: counts update correctly as blocks are added TEST_CASE("Counts update correctly as blocks are added", "[US5][persistence]") { - auto dir = create_test_dir("T049"); + auto dir = TestHelpers::createTestDir("T049"); { auto cfg = TestHelpers::defaultConsensusConfig(); Blockchain bc(dir, cfg); @@ -283,13 +249,13 @@ TEST_CASE("Counts update correctly as blocks are added", "[US5][persistence]") bc.publish("test", "k2", "data", {"k2"}); REQUIRE(bc.getChainLength() == 3); } - cleanup_test_dir(dir); + TestHelpers::cleanupTestDir(dir); } // T060a: chunk auto-save completes within 2 seconds on a 100-block chunk TEST_CASE("Chunk auto-save completes within 2 seconds", "[US5][performance]") { - auto dir = create_test_dir("T060a"); + auto dir = TestHelpers::createTestDir("T060a"); { auto cfg = TestHelpers::defaultConsensusConfig(); Blockchain bc(dir, cfg); @@ -307,13 +273,13 @@ TEST_CASE("Chunk auto-save completes within 2 seconds", "[US5][performance]") REQUIRE(elapsed < std::chrono::seconds(2)); REQUIRE(std::filesystem::exists(dir / "chunk_000000.dat")); } - cleanup_test_dir(dir); + TestHelpers::cleanupTestDir(dir); } // --- T015: Chunk retention during multi-access operations --- TEST_CASE("ChunkRetainGuard prevents freeChunk from clearing retained chunks", "[US3][persistence]") { - auto dir = create_test_dir("T015_retain"); + auto dir = TestHelpers::createTestDir("T015_retain"); { auto cfg = TestHelpers::defaultConsensusConfig(); Blockchain bc(dir, cfg); @@ -339,11 +305,11 @@ TEST_CASE("ChunkRetainGuard prevents freeChunk from clearing retained chunks", " // releaseChunks frees retained chunks bc.releaseChunks(); } - cleanup_test_dir(dir); + TestHelpers::cleanupTestDir(dir); } TEST_CASE("ChunkRetainGuard RAII releases chunks on scope exit", "[US3][persistence]") { - auto dir = create_test_dir("T015_raii"); + auto dir = TestHelpers::createTestDir("T015_raii"); { auto cfg = TestHelpers::defaultConsensusConfig(); Blockchain bc(dir, cfg); @@ -365,5 +331,5 @@ TEST_CASE("ChunkRetainGuard RAII releases chunks on scope exit", "[US3][persiste // Guard released — chunks freed SUCCEED("ChunkRetainGuard RAII cleanup completed"); } - cleanup_test_dir(dir); + TestHelpers::cleanupTestDir(dir); } diff --git a/tests/consensus_tests.cpp b/tests/consensus_tests.cpp index ba2461a..00c0a78 100644 --- a/tests/consensus_tests.cpp +++ b/tests/consensus_tests.cpp @@ -9,30 +9,6 @@ #include #include -// Helper: create a block with a valid PoW for the given difficulty -static Block mineBlock(size_t index, uint64_t timestamp, const std::string &prevHash, - const std::string &data, uint32_t difficulty) -{ - StreamEntry entry; - entry.stream = "test"; - entry.key = "k"; - entry.data = data; - - Block b; - b.index = index; - b.timestamp = timestamp; - b.entries = {entry}; - b.prevHash = prevHash; - b.difficulty = difficulty; - b.nonce = 0; - b.hash = b.calculateHash(); - while (!checkLeadingZeroBits(b.hash, difficulty)) { - b.nonce++; - b.hash = b.calculateHash(); - } - return b; -} - // ========================================================================== // checkLeadingZeroBits tests // ========================================================================== @@ -85,7 +61,7 @@ TEST_CASE("isValidNewBlock accepts block with valid PoW", "[Consensus][US1]") auto now = static_cast(std::time(nullptr)); Block genesis(0, 0, "", {}, 0, 0); - Block valid = mineBlock(1, now, genesis.hash, "test data", 1); + Block valid = TestHelpers::mineTestBlock(1, now, genesis.hash, "test data", 1); REQUIRE(IBlockchain::isValidNewBlock(valid, genesis, config)); } @@ -129,7 +105,7 @@ TEST_CASE("isValidNewBlock rejects block with incorrect prevHash", "[Consensus][ auto now = static_cast(std::time(nullptr)); Block genesis(0, 0, "", {}, 0, 0); - Block bad = mineBlock(1, now, "wrong_prev_hash", "test data", 1); + Block bad = TestHelpers::mineTestBlock(1, now, "wrong_prev_hash", "test data", 1); REQUIRE_FALSE(IBlockchain::isValidNewBlock(bad, genesis, config)); } @@ -162,7 +138,7 @@ TEST_CASE("isValidNewBlock rejects block with future timestamp", "[Consensus][US Block genesis(0, 0, "", {}, 0, 0); // Create block with timestamp 200s in the future (> 120s allowed) - Block futureBlock = mineBlock(1, now + 200, genesis.hash, "future data", 1); + Block futureBlock = TestHelpers::mineTestBlock(1, now + 200, genesis.hash, "future data", 1); REQUIRE_FALSE(IBlockchain::isValidNewBlock(futureBlock, genesis, config)); } @@ -177,7 +153,7 @@ TEST_CASE("isValidNewBlock rejects block below minDifficulty", "[Consensus][US1] Block genesis(0, 0, "", {}, 0, 0); // Mine a block with difficulty=1 but config requires minDifficulty=4 - Block lowDiff = mineBlock(1, now, genesis.hash, "low diff", 1); + Block lowDiff = TestHelpers::mineTestBlock(1, now, genesis.hash, "low diff", 1); REQUIRE_FALSE(IBlockchain::isValidNewBlock(lowDiff, genesis, config)); } @@ -261,8 +237,8 @@ TEST_CASE("Longer valid chain replaces shorter chain", "[Consensus][US3]") Block genesis = bc.getBlockByIndex(0); auto now = static_cast(std::time(nullptr)); - Block b1 = mineBlock(1, now, genesis.hash, "candidate block 1", 1); - Block b2 = mineBlock(2, now + 1, b1.hash, "candidate block 2", 1); + Block b1 = TestHelpers::mineTestBlock(1, now, genesis.hash, "candidate block 1", 1); + Block b2 = TestHelpers::mineTestBlock(2, now + 1, b1.hash, "candidate block 2", 1); std::vector candidate = {genesis, b1, b2}; @@ -288,7 +264,7 @@ TEST_CASE("Shorter chain does not replace longer chain", "[Consensus][US3]") // Try to replace with a shorter chain (just genesis + 1 block) Block genesis = bc.getBlockByIndex(0); auto now = static_cast(std::time(nullptr)); - Block b1 = mineBlock(1, now, genesis.hash, "short chain", 1); + Block b1 = TestHelpers::mineTestBlock(1, now, genesis.hash, "short chain", 1); std::vector candidate = {genesis, b1}; bc.replaceChain(candidate); @@ -306,10 +282,10 @@ TEST_CASE("Longer chain with invalid block is rejected", "[Consensus][US3]") Block genesis = bc.getBlockByIndex(0); auto now = static_cast(std::time(nullptr)); - Block b1 = mineBlock(1, now, genesis.hash, "valid block", 1); + Block b1 = TestHelpers::mineTestBlock(1, now, genesis.hash, "valid block", 1); // Create an invalid block (wrong prevHash) - Block b2_invalid = mineBlock(2, now + 1, "wrong_hash", "invalid block", 1); + Block b2_invalid = TestHelpers::mineTestBlock(2, now + 1, "wrong_hash", "invalid block", 1); std::vector candidate = {genesis, b1, b2_invalid}; bc.replaceChain(candidate); @@ -332,7 +308,7 @@ TEST_CASE("Chain reorg deeper than maxReorgDepth is rejected", "[Consensus][US3] std::vector candidate = {genesis}; for (int i = 1; i <= 5; i++) { Block prev = candidate.back(); - candidate.push_back(mineBlock(i, now + i, prev.hash, "deep_" + std::to_string(i), 1)); + candidate.push_back(TestHelpers::mineTestBlock(i, now + i, prev.hash, "deep_" + std::to_string(i), 1)); } bc.replaceChain(candidate); @@ -354,8 +330,8 @@ TEST_CASE("keyIndexMap is rebuilt after chain replacement", "[Consensus][US3]") // Build longer candidate chain Block genesis = bc.getBlockByIndex(0); auto now = static_cast(std::time(nullptr)); - Block b1 = mineBlock(1, now, genesis.hash, "new data 1", 1); - Block b2 = mineBlock(2, now + 1, b1.hash, "new data 2", 1); + Block b1 = TestHelpers::mineTestBlock(1, now, genesis.hash, "new data 1", 1); + Block b2 = TestHelpers::mineTestBlock(2, now + 1, b1.hash, "new data 2", 1); std::vector candidate = {genesis, b1, b2}; bc.replaceChain(candidate); @@ -388,7 +364,7 @@ TEST_CASE("Difficulty increases when blocks mined faster than target", "[Consens for (int i = 1; i <= 6; i++) { Block prev = fastChain.back(); // 1 second apart (much faster than 10s target) - fastChain.push_back(mineBlock(i, now + i, prev.hash, "fast_" + std::to_string(i), 1)); + fastChain.push_back(TestHelpers::mineTestBlock(i, now + i, prev.hash, "fast_" + std::to_string(i), 1)); } bc.replaceChain(fastChain); @@ -415,7 +391,7 @@ TEST_CASE("Difficulty decreases when blocks mined slower than target", "[Consens for (int i = 1; i <= 6; i++) { Block prev = slowChain.back(); // 100 seconds apart in the past (much slower than 10s target) - slowChain.push_back(mineBlock(i, now - 700 + i * 100, prev.hash, "slow_" + std::to_string(i), 4)); + slowChain.push_back(TestHelpers::mineTestBlock(i, now - 700 + i * 100, prev.hash, "slow_" + std::to_string(i), 4)); } bc.replaceChain(slowChain); @@ -442,7 +418,7 @@ TEST_CASE("Difficulty does not change by more than maxAdjustmentFactor", "[Conse for (int i = 1; i <= 6; i++) { Block prev = extremeChain.back(); // Extremely fast: all same timestamp in the past - extremeChain.push_back(mineBlock(i, now - 10, prev.hash, "extreme_" + std::to_string(i), 4)); + extremeChain.push_back(TestHelpers::mineTestBlock(i, now - 10, prev.hash, "extreme_" + std::to_string(i), 4)); } bc.replaceChain(extremeChain); @@ -470,7 +446,7 @@ TEST_CASE("Difficulty clamped to min/max range", "[Consensus][US4]") std::vector chain = {genesis}; for (int i = 1; i <= 6; i++) { Block prev = chain.back(); - chain.push_back(mineBlock(i, now + i, prev.hash, "clamped_" + std::to_string(i), 1)); + chain.push_back(TestHelpers::mineTestBlock(i, now + i, prev.hash, "clamped_" + std::to_string(i), 1)); } bc.replaceChain(chain); @@ -497,7 +473,7 @@ TEST_CASE("Difficulty stays at minimum when slow and already at minimum", "[Cons std::vector slowChain = {genesis}; for (int i = 1; i <= 6; i++) { Block prev = slowChain.back(); - slowChain.push_back(mineBlock(i, now - 700 + i * 100, prev.hash, "min_" + std::to_string(i), 1)); + slowChain.push_back(TestHelpers::mineTestBlock(i, now - 700 + i * 100, prev.hash, "min_" + std::to_string(i), 1)); } bc.replaceChain(slowChain); @@ -526,7 +502,7 @@ TEST_CASE("getDifficultyForHeight returns cached result on second call", "[Conse // Build a chain with >5 blocks for at least one adjustment boundary for (int i = 1; i <= 10; i++) { Block prev = bc.getBlockByIndex(bc.getChainBlockCount() - 1); - Block b = mineBlock(i, now + i * 10, prev.hash, "cache_" + std::to_string(i), 1); + Block b = TestHelpers::mineTestBlock(i, now + i * 10, prev.hash, "cache_" + std::to_string(i), 1); bc.appendBlock(b); } @@ -559,7 +535,7 @@ TEST_CASE("Difficulty cache invalidated on replaceChain", "[Consensus][US3]") { // Build initial chain for (int i = 1; i <= 6; i++) { Block prev = bc.getBlockByIndex(bc.getChainBlockCount() - 1); - Block b = mineBlock(i, now + i * 10, prev.hash, "orig_" + std::to_string(i), 1); + Block b = TestHelpers::mineTestBlock(i, now + i * 10, prev.hash, "orig_" + std::to_string(i), 1); bc.appendBlock(b); } @@ -571,7 +547,7 @@ TEST_CASE("Difficulty cache invalidated on replaceChain", "[Consensus][US3]") { std::vector candidate = {genesis}; for (int i = 1; i <= 8; i++) { Block prev = candidate.back(); - candidate.push_back(mineBlock(i, now + i * 5, prev.hash, "new_" + std::to_string(i), 1)); + candidate.push_back(TestHelpers::mineTestBlock(i, now + i * 5, prev.hash, "new_" + std::to_string(i), 1)); } bc.replaceChain(candidate); diff --git a/tests/sync_tests.cpp b/tests/sync_tests.cpp index 7a4154a..460f448 100644 --- a/tests/sync_tests.cpp +++ b/tests/sync_tests.cpp @@ -8,51 +8,13 @@ #include "../src/network/SyncMessages.hpp" #include "../src/network/PacketHeader.hpp" #include "../src/utils.hpp" +#include "TestHelpers.hpp" #include #include #include #include #include -// Helper: create a block with valid PoW for the given difficulty -static Block mineTestBlock(size_t index, uint64_t timestamp, const std::string &prevHash, - const std::string &data, uint32_t difficulty) -{ - StreamEntry entry; - entry.stream = "test"; - entry.key = "k"; - entry.data = data; - - Block b; - b.index = index; - b.timestamp = timestamp; - b.entries = {entry}; - b.prevHash = prevHash; - b.difficulty = difficulty; - b.nonce = 0; - b.hash = b.calculateHash(); - while (difficulty > 0 && !checkLeadingZeroBits(b.hash, difficulty)) { - b.nonce++; - b.hash = b.calculateHash(); - } - return b; -} - -// Helper: build a valid chain of N blocks -static std::vector buildValidChain(size_t numBlocks, uint32_t difficulty = 1) -{ - std::vector chain; - Block genesis(0, 0, "", {}, 0, 0); - chain.push_back(genesis); - - for (size_t i = 1; i < numBlocks; i++) { - Block b = mineTestBlock(i, static_cast(i * 10), chain.back().hash, - "block " + std::to_string(i), difficulty); - chain.push_back(b); - } - return chain; -} - // ========================================================================== // SyncMessages serialization tests // ========================================================================== @@ -81,7 +43,7 @@ TEST_CASE("SyncResponse serialization round-trip", "[Sync][Setup]") SyncResponse response; response.total_chain_height = 200; response.chunk_index = 1; - response.blocks = buildValidChain(3); + response.blocks = TestHelpers::buildValidChain(3, 1); std::stringstream ss; { @@ -236,7 +198,7 @@ TEST_CASE("PeerClient receives BLOCKCHAIN_RESPONSE and validates blocks", "[Sync config.minDifficulty = 1; // Build a chain of 5 blocks to simulate what a peer would send - auto chain = buildValidChain(5, 1); + auto chain = TestHelpers::buildValidChain(5, 1); SyncResponse response; response.total_chain_height = 5; @@ -312,7 +274,7 @@ TEST_CASE("PeerClient sends BLOCKCHAIN_QUERY with height > 1 for incremental syn response.chunk_index = 0; // Peer sends only the blocks the client is missing - auto full_chain = buildValidChain(10, 1); + auto full_chain = TestHelpers::buildValidChain(10, 1); for (size_t i = query.local_chain_height; i < full_chain.size(); i++) { response.blocks.push_back(full_chain[i]); } @@ -349,7 +311,7 @@ TEST_CASE("PeerClient rejects chunk with invalid hash", "[Sync][US3]") config.initialDifficulty = 1; config.minDifficulty = 1; - auto chain = buildValidChain(5, 1); + auto chain = TestHelpers::buildValidChain(5, 1); // Tamper with block 3's hash chain[3].hash = "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; @@ -374,7 +336,7 @@ TEST_CASE("PeerClient rejects chunk with insufficient PoW difficulty", "[Sync][U ConsensusConfig config; config.minDifficulty = 4; - auto chain = buildValidChain(3, 1); // mined at difficulty 1 + auto chain = TestHelpers::buildValidChain(3, 1); // mined at difficulty 1 // Block at difficulty 1 should fail when config requires minDifficulty=4 bool valid = true; @@ -401,7 +363,7 @@ TEST_CASE("PeerClient accepts valid longer chain", "[Sync][US3]") REQUIRE(bc.getChainBlockCount() == 1); // Build a valid longer chain - auto longer_chain = buildValidChain(5, 1); + auto longer_chain = TestHelpers::buildValidChain(5, 1); bc.replaceChain(longer_chain); REQUIRE(bc.getChainBlockCount() == 5); @@ -421,7 +383,7 @@ TEST_CASE("PeerClient keeps local chain when peer chain is same length", "[Sync] REQUIRE(bc.getChainBlockCount() == 3); // Build candidate chain of same length - auto same_length_chain = buildValidChain(3, 1); + auto same_length_chain = TestHelpers::buildValidChain(3, 1); // replaceChain should reject since not longer bc.replaceChain(same_length_chain); @@ -463,7 +425,7 @@ TEST_CASE("Already-persisted chunks preserved when connection drops", "[Sync][US Blockchain bc(".", config); // Simulate having received and persisted one chunk of blocks - auto chain = buildValidChain(5, 1); + auto chain = TestHelpers::buildValidChain(5, 1); bc.replaceChain(chain); REQUIRE(bc.getChainBlockCount() == 5); @@ -538,9 +500,9 @@ TEST_CASE("handle_sync_response appends new blocks", "[Sync][US1]") // Build a valid chain extending from genesis Block genesis = bc.getBlockByIndex(0); - Block b1 = mineTestBlock(1, 100, genesis.hash, "sync_b1", 0); - Block b2 = mineTestBlock(2, 200, b1.hash, "sync_b2", 0); - Block b3 = mineTestBlock(3, 300, b2.hash, "sync_b3", 0); + Block b1 = TestHelpers::mineTestBlock(1, 100, genesis.hash, "sync_b1", 0); + Block b2 = TestHelpers::mineTestBlock(2, 200, b1.hash, "sync_b2", 0); + Block b3 = TestHelpers::mineTestBlock(3, 300, b2.hash, "sync_b3", 0); // Simulate what the fixed handler should do: append each new block bc.appendBlock(b1); @@ -568,14 +530,14 @@ TEST_CASE("handle_sync_response skips already-known blocks", "[Sync][US1]") // Add some blocks to the local chain Block genesis = bc.getBlockByIndex(0); - Block b1 = mineTestBlock(1, 100, genesis.hash, "local_b1", 0); - Block b2 = mineTestBlock(2, 200, b1.hash, "local_b2", 0); + Block b1 = TestHelpers::mineTestBlock(1, 100, genesis.hash, "local_b1", 0); + Block b2 = TestHelpers::mineTestBlock(2, 200, b1.hash, "local_b2", 0); bc.appendBlock(b1); bc.appendBlock(b2); REQUIRE(bc.getChainBlockCount() == 3); // Simulate a sync response that overlaps: blocks 1, 2 (known) + 3 (new) - Block b3 = mineTestBlock(3, 300, b2.hash, "sync_b3", 0); + Block b3 = TestHelpers::mineTestBlock(3, 300, b2.hash, "sync_b3", 0); // The handler should skip blocks below local_height and only append new ones size_t local_height = bc.getChainBlockCount(); @@ -610,13 +572,13 @@ TEST_CASE("handle_sync_response aborts on overlap hash mismatch", "[Sync][US1]") // Build local chain: genesis + b1 Block genesis = bc.getBlockByIndex(0); - Block b1 = mineTestBlock(1, 100, genesis.hash, "local_b1", 0); + Block b1 = TestHelpers::mineTestBlock(1, 100, genesis.hash, "local_b1", 0); bc.appendBlock(b1); REQUIRE(bc.getChainBlockCount() == 2); // Build a response with a different block at index 1 (hash mismatch) - Block fake_b1 = mineTestBlock(1, 999, genesis.hash, "fake_data", 0); - Block fake_b2 = mineTestBlock(2, 1000, fake_b1.hash, "fake_b2", 0); + Block fake_b1 = TestHelpers::mineTestBlock(1, 999, genesis.hash, "fake_data", 0); + Block fake_b2 = TestHelpers::mineTestBlock(2, 1000, fake_b1.hash, "fake_b2", 0); // The handler should detect the overlap mismatch and abort size_t local_height = bc.getChainBlockCount(); diff --git a/tests/utils_tests.cpp b/tests/utils_tests.cpp index eed4885..1f4895b 100644 --- a/tests/utils_tests.cpp +++ b/tests/utils_tests.cpp @@ -1,5 +1,9 @@ #include #include "../src/utils.hpp" +#include "../src/network/PacketSerializer.hpp" +#include "../src/Block.hpp" +#include +#include TEST_CASE("parsePeerKey rejects port 0", "[utils][parsePeerKey]") { REQUIRE_THROWS_AS(parsePeerKey("host:0"), std::invalid_argument); @@ -34,3 +38,27 @@ TEST_CASE("parsePeerKey accepts hostname with port 1", "[utils][parsePeerKey]") REQUIRE(host == "host"); REQUIRE(port == 1); } + +TEST_CASE("serialize_packet produces correct header and payload", "[utils][PacketSerializer]") { + Block b; + b.index = 42; + b.prevHash = "abc"; + + constexpr uint64_t ptype = 99; + auto [header_data, payload] = serialize_packet(b, ptype); + + REQUIRE(header_data.size() == sizeof(PacketHeader)); + + PacketHeader hdr; + std::memcpy(&hdr, header_data.data(), sizeof(hdr)); + REQUIRE(hdr.type == ptype); + REQUIRE(hdr.length == payload.size()); + + // Verify payload deserializes back to an equivalent Block + std::istringstream iss(payload); + boost::archive::binary_iarchive ia(iss); + Block restored; + ia >> restored; + REQUIRE(restored.index == b.index); + REQUIRE(restored.prevHash == b.prevHash); +}