Skip to content

feat: add minimal unopinionated reqresp v1 package#66

Open
samcm wants to merge 13 commits intomasterfrom
feat/unoppinionated-reqresp
Open

feat: add minimal unopinionated reqresp v1 package#66
samcm wants to merge 13 commits intomasterfrom
feat/unoppinionated-reqresp

Conversation

@samcm
Copy link
Copy Markdown
Member

@samcm samcm commented Jul 4, 2025

Summary

  • Adds a new minimal, unopinionated request/response package in pkg/consensus/mimicry/p2p/reqresp/v1
  • Supports both single-response and chunked-response protocols
  • Uses Go generics for type safety without imposing specific encoding formats
  • No external dependencies on Prysm or ZRNT types

Key Features

  • Generic interfaces: Core abstractions use generics for compile-time type safety
  • Chunked protocol support: Enables protocols that send multiple response chunks (needed for BeaconBlocksByRangeV2)
  • Pluggable encoding: Users provide their own encoder implementation (JSON, SSZ, protobuf, etc.)
  • Optional compression: Compression is optional and pluggable
  • Clean separation: Handler logic, client logic, and service management are clearly separated

Implementation Details

The package includes:

  • interface.go: Core interfaces and abstractions
  • types.go: Basic types, error definitions, and configuration structs
  • handler.go: Generic handler implementation with request/response processing
  • chunked_handler.go: Support for protocols that send multiple response chunks
  • client.go: Client implementation with retry logic and chunked response support
  • reqresp.go: Main service implementation that ties everything together
  • protocols.go: Helper types for creating protocols
  • example_test.go: Usage examples showing JSON encoding, custom protocols, and chunked responses

Test Plan

  • Package compiles successfully
  • Examples demonstrate proper usage
  • No go vet issues
  • Unit tests to be added in follow-up PR (targeting >70% coverage)

🤖 Generated with Claude Code

- Add core interfaces for request/response communication
- Support both single and chunked response protocols
- Provide generic type-safe APIs without external dependencies
- Include flexible encoding/compression abstractions
- Add comprehensive examples demonstrating usage

This implementation provides a clean, minimal API for libp2p request/response
protocols without being opinionated about encoding formats or protocol details.
It supports the chunked response pattern needed for protocols like
BeaconBlocksByRangeV2.

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
@samcm samcm requested a review from mattevans as a code owner July 4, 2025 07:23
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 4, 2025

Codecov Report

Attention: Patch coverage is 0.90909% with 327 lines in your changes missing coverage. Please review.

Project coverage is 39.47%. Comparing base (1583744) to head (99071a5).
Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
pkg/consensus/mimicry/p2p/reqresp/v1/reqresp.go 0.00% 189 Missing ⚠️
.../consensus/mimicry/p2p/reqresp/v1/eth/protocols.go 0.00% 77 Missing ⚠️
...ensus/mimicry/p2p/reqresp/v1/ssz_snappy_encoder.go 0.00% 30 Missing ⚠️
pkg/consensus/mimicry/p2p/reqresp/v1/protocols.go 0.00% 26 Missing ⚠️
pkg/consensus/mimicry/crawler/crawler.go 37.50% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #66      +/-   ##
==========================================
+ Coverage   39.43%   39.47%   +0.04%     
==========================================
  Files          59       63       +4     
  Lines        5942     6275     +333     
==========================================
+ Hits         2343     2477     +134     
- Misses       3354     3529     +175     
- Partials      245      269      +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

samcm and others added 12 commits July 4, 2025 17:44
- Remove global encoder/compressor from service config
- Add HandlerOptions parameter to RegisterProtocol functions
- Add SendRequestWithOptions for per-request encoding
- Update examples to show protocol-specific encoding
- Improve middleware example to demonstrate actual usage

This allows different protocols to use different encoding strategies:
- JSON for metadata protocols
- SSZ for consensus data
- Different compression per protocol

Breaking changes:
- RegisterProtocol now requires HandlerOptions parameter
- ClientConfig no longer has Encoder/Compressor fields

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
- Add tests for client.go covering SendRequest, retry logic, timeouts, and chunked requests
- Add tests for handler.go covering request handling, validation, and error responses
- Add tests for chunked_handler.go covering chunked responses and ChunkedResponseWriter
- Add tests for reqresp.go covering service lifecycle and concurrent operations
- Add tests for types.go covering Status, error constants, and configurations
- Create comprehensive mock implementations for host.Host, network.Stream, and other interfaces
- Achieve 72.6% test coverage, exceeding the >70% target

Test coverage breakdown:
- client.go: High coverage for core functionality
- handler.go: Excellent coverage (94.7% for HandleStream)
- chunked_handler.go: Excellent coverage (95% for HandleStream)
- types.go: 100% coverage for key functions
- reqresp.go: Good coverage for service operations
- Add unit tests for all major components
- Add mock implementations for libp2p interfaces
- Add panic recovery to handlers for robustness
- Add proper timeout handling with context
- Fix empty request validation
- Fix integration test race conditions
- All tests now pass successfully

Coverage highlights:
- Total package: 77.9%
- Handler.HandleStream: 94.7%
- ChunkedHandler.HandleStream: 95.0%
- Client.SendRequestWithOptions: 95.5%

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
- Add blank line before return statements (nlreturn)
- Add blank line before if statements after assignments (wsl)
- All linting issues now resolved

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
feat: support per-protocol encoding in reqresp v1
Add compile-time safe protocol creation for Ethereum consensus layer protocols.
The eth package provides factory functions that validate protocol IDs at compile
time without being opinionated about message types or encoders.

- Factory functions for all Ethereum consensus protocols
- No predefined message types (users provide their own)
- No default protocol instances
- Comprehensive examples showing usage patterns

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…aces

- Merged Encoder and Compressor into single NetworkEncoder interface
- Updated all protocol constructors to use NetworkEncoder
- Removed unnecessary abstractions and simplified architecture
- Added SSZSnappyEncoder implementation as example
- Updated all tests and examples to use new API
- Removed unused fields and imports

This change better reflects how Ethereum protocols work (always using
SSZ+Snappy together) and significantly simplifies the API.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Removed empty Config struct that served no purpose
- Deleted broken example files that provided no value
- Simplified protocol abstractions into single SimpleProtocol type
- Merged ChunkedProtocol interface into Protocol with IsChunked() method
- Removed inheritance pattern in favor of simple struct with boolean field

The package is now much cleaner with only essential functionality and
no unnecessary abstractions or broken examples.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add tests for all core functionality with 87.3% coverage
- Add tests for protocol implementations with 100% coverage
- Test service lifecycle, handler registration, stream handling
- Test error scenarios and concurrent operations
- Test SSZ+Snappy encoder implementation
- Test all Ethereum protocol constructors

All tests pass successfully, exceeding the 80% coverage target.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants