diff --git a/.ai/CODE_REVIEW.md b/.ai/CODE_REVIEW.md new file mode 100644 index 0000000000..e4da3b22d5 --- /dev/null +++ b/.ai/CODE_REVIEW.md @@ -0,0 +1,277 @@ +# Lighthouse Code Review Guidelines + +Code review guidelines based on patterns from Lighthouse maintainers. + +## Core Principles + +- **Correctness** over clever code +- **Clarity** through good documentation and naming +- **Safety** through proper error handling and panic avoidance +- **Maintainability** for long-term health + +## Critical: Consensus Crate (`consensus/` excluding `types/`) + +**Extra scrutiny required** - bugs here cause consensus failures. + +### Requirements + +1. **Safe Math Only** + ```rust + // NEVER + let result = a + b; + + // ALWAYS + let result = a.saturating_add(b); + // or use safe_arith crate + let result = a.safe_add(b)?; + ``` + +2. **Zero Panics** + - No `.unwrap()`, `.expect()`, array indexing `[i]` + - Return `Result` or `Option` instead + +3. **Deterministic Behavior** + - Identical results across all platforms + - No undefined behavior + +## Panic Avoidance (All Code) + +```rust +// NEVER at runtime +let value = option.unwrap(); +let item = array[1]; + +// ALWAYS +let value = option.ok_or(Error::Missing)?; +let item = array.get(1)?; + +// Only acceptable during startup for CLI/config validation +let flag = matches.get_one::("flag") + .expect("Required due to clap validation"); +``` + +## Code Clarity + +### Variable Naming +```rust +// BAD - ambiguous +let bb = ...; +let bl = ...; + +// GOOD - clear +let beacon_block = ...; +let blob = ...; +``` + +### Comments +- Explain the "why" not just the "what" +- All `TODO` comments must link to a GitHub issue +- Remove dead/commented-out code + +## Error Handling + +### Don't Silently Swallow Errors +```rust +// BAD +self.store.get_info().unwrap_or(None) + +// GOOD +self.store.get_info().unwrap_or_else(|e| { + error!(self.log, "Failed to read info"; "error" => ?e); + None +}) +``` + +### Check Return Values +Ask: "What happens if this returns `Ok(Failed)`?" Don't ignore results that might indicate failure. + +## Performance & Concurrency + +### Lock Safety +- Document lock ordering requirements +- Keep lock scopes narrow +- Seek detailed review for lock-related changes +- Use `try_read` when falling back to an alternative is acceptable +- Use blocking `read` when alternative is more expensive (e.g., state reconstruction) + +### Async Patterns +```rust +// NEVER block in async context +async fn handler() { + expensive_computation(); // blocks runtime +} + +// ALWAYS spawn blocking +async fn handler() { + tokio::task::spawn_blocking(|| expensive_computation()).await?; +} +``` + +### Rayon +- Use scoped rayon pools from beacon processor +- Avoid global thread pool (causes CPU oversubscription) + +## Review Process + +### Focus on Actionable Issues + +**Limit to 3-5 key comments.** Prioritize: +1. Correctness issues - bugs, race conditions, panics +2. Missing test coverage - especially edge cases +3. Complex logic needing documentation +4. API design concerns + +**Don't comment on:** +- Minor style issues +- Things caught by CI (formatting, linting) +- Nice-to-haves that aren't important + +### Keep Comments Natural and Minimal + +**Tone**: Natural and conversational, not robotic. + +**Good review comment:** +``` +Missing test coverage for the None blobs path. The existing test at +`store_tests.rs:2874` still provides blobs. Should add a test passing +None to verify backfill handles this correctly. +``` + +**Good follow-up after author addresses comments:** +``` +LGTM, thanks! +``` +or +``` +Thanks for the updates, looks good! +``` + +**Avoid:** +- Checklists or structured formatting (✅ Item 1 fixed...) +- Repeating what was fixed (makes it obvious it's AI-generated) +- Headers, subsections, "Summary" sections +- Verbose multi-paragraph explanations + +### Use Natural Language + +``` +BAD (prescriptive): +"This violates coding standards which strictly prohibit runtime panics." + +GOOD (conversational): +"Should we avoid `.expect()` here? This gets called in hot paths and +we typically try to avoid runtime panics outside of startup." +``` + +### Verify Before Commenting + +- If CI passes, trust it - types/imports must exist +- Check the full diff, not just visible parts +- Ask for verification rather than asserting things are missing + +## Common Review Patterns + +### Fork-Specific Changes +- Verify production fork code path unchanged +- Check SSZ compatibility (field order) +- Verify rollback/error paths handle edge cases + +### API Design +- Constructor signatures should be consistent +- Avoid `Option` parameters when value is always required + +### Concurrency +- Lock ordering documented? +- Potential deadlocks? +- Race conditions? + +### Error Handling +- Errors logged? +- Edge cases handled? +- Context provided with errors? + +## Deep Review Techniques + +### Verify Against Specifications +- Read the actual spec in `./consensus-specs/` +- Compare formulas exactly +- Check constant values match spec definitions + +### Trace Data Flow End-to-End +For new config fields: +1. Config file - Does YAML contain the field? +2. Config struct - Is it parsed with serde attributes? +3. apply_to_chain_spec - Is it actually applied? +4. Runtime usage - Used correctly everywhere? + +### Check Error Handling Fallbacks +Examine every `.unwrap_or()`, `.unwrap_or_else()`: +- If the fallback triggers, does code behave correctly? +- Does it silently degrade or fail loudly? + +### Look for Incomplete Migrations +When a PR changes a pattern across the codebase: +- Search for old pattern - all occurrences updated? +- Check test files - often lag behind implementation + +## Architecture & Design + +### Avoid Dependency Bloat +- Question whether imports add unnecessary dependencies +- Consider feature flags for optional functionality +- Large imports when only primitives are needed may warrant a `core` or `primitives` feature + +### Schema Migrations +- Database schema changes require migrations +- Don't forget to add migration code when changing stored types +- Review pattern: "Needs a schema migration" + +### Backwards Compatibility +- Consider existing users when changing behavior +- Document breaking changes clearly +- Prefer additive changes when possible + +## Anti-Patterns to Avoid + +### Over-Engineering +- Don't add abstractions until needed +- Keep solutions simple and focused +- "Three similar lines of code is better than a premature abstraction" + +### Unnecessary Complexity +- Avoid feature flags for simple changes +- Don't add fallbacks for scenarios that can't happen +- Trust internal code and framework guarantees + +### Premature Optimization +- Optimize hot paths based on profiling, not assumptions +- Document performance considerations but don't over-optimize + +### Hiding Important Information +- Don't use generic variable names when specific ones are clearer +- Don't skip logging just to keep code shorter +- Don't omit error context + +## Design Principles + +### Simplicity First +Question every layer of abstraction: +- Is this `Arc` needed, or is the inner type already `Clone`? +- Is this `Mutex` needed, or can ownership be restructured? +- Is this wrapper type adding value or just indirection? + +If you can't articulate why a layer of abstraction exists, it probably shouldn't. + +### High Cohesion +Group related state and behavior together. If two fields are always set together, used together, and invalid without each other, they belong in a struct. + +## Before Approval Checklist + +- [ ] No panics: No `.unwrap()`, `.expect()`, unchecked array indexing +- [ ] Consensus safe: If touching consensus crate, all arithmetic is safe +- [ ] Errors logged: Not silently swallowed +- [ ] Clear naming: Variable names are unambiguous +- [ ] TODOs linked: All TODOs have GitHub issue links +- [ ] Tests present: Non-trivial changes have tests +- [ ] Lock safety: Lock ordering is safe and documented +- [ ] No blocking: Async code doesn't block runtime diff --git a/.ai/DEVELOPMENT.md b/.ai/DEVELOPMENT.md new file mode 100644 index 0000000000..1204f21ead --- /dev/null +++ b/.ai/DEVELOPMENT.md @@ -0,0 +1,200 @@ +# Lighthouse Development Guide + +Development patterns, commands, and architecture for AI assistants and contributors. + +## Development Commands + +**Important**: Always branch from `unstable` and target `unstable` when creating pull requests. + +### Building + +- `make install` - Build and install Lighthouse in release mode +- `make install-lcli` - Build and install `lcli` utility +- `cargo build --release` - Standard release build +- `cargo build --bin lighthouse --features "gnosis,slasher-lmdb"` - Build with specific features + +### Testing + +- `make test` - Full test suite in release mode +- `make test-release` - Run tests using nextest (faster parallel runner) +- `cargo nextest run -p ` - Run tests for specific package (preferred for iteration) +- `cargo nextest run -p ` - Run individual test +- `FORK_NAME=electra cargo nextest run -p beacon_chain` - Run tests for specific fork +- `make test-ef` - Ethereum Foundation test vectors + +**Fork-specific testing**: `beacon_chain` and `http_api` tests support fork-specific testing via `FORK_NAME` env var when `beacon_chain/fork_from_env` feature is enabled. + +**Note**: Full test suite takes ~20 minutes. Prefer targeted tests when iterating. + +### Linting + +- `make lint` - Run Clippy with project rules +- `make lint-full` - Comprehensive linting including tests +- `cargo fmt --all && make lint-fix` - Format and fix linting issues +- `cargo sort` - Sort dependencies (enforced on CI) + +## Architecture Overview + +Lighthouse is a modular Ethereum consensus client with two main components: + +### Beacon Node (`beacon_node/`) + +- Main consensus client syncing with Ethereum network +- Beacon chain state transition logic (`beacon_node/beacon_chain/`) +- Networking, storage, P2P communication +- HTTP API for validator clients +- Entry point: `beacon_node/src/lib.rs` + +### Validator Client (`validator_client/`) + +- Manages validator keystores and duties +- Block proposals, attestations, sync committee duties +- Slashing protection and doppelganger detection +- Entry point: `validator_client/src/lib.rs` + +### Key Subsystems + +| Subsystem | Location | Purpose | +|-----------|----------|---------| +| Consensus Types | `consensus/types/` | Core data structures, SSZ encoding | +| Storage | `beacon_node/store/` | Hot/cold database (LevelDB, RocksDB, REDB backends) | +| Networking | `beacon_node/lighthouse_network/` | Libp2p, gossipsub, discovery | +| Fork Choice | `consensus/fork_choice/` | Proto-array fork choice | +| Execution Layer | `beacon_node/execution_layer/` | EL client integration | +| Slasher | `slasher/` | Optional slashing detection | + +### Utilities + +- `account_manager/` - Validator account management +- `lcli/` - Command-line debugging utilities +- `database_manager/` - Database maintenance tools + +## Code Quality Standards + +### Panic Avoidance (Critical) + +**Panics should be avoided at all costs.** + +```rust +// NEVER at runtime +let value = some_result.unwrap(); +let item = array[1]; + +// ALWAYS prefer +let value = some_result?; +let item = array.get(1)?; + +// Only acceptable during startup +let config = matches.get_one::("flag") + .expect("Required due to clap validation"); +``` + +### Consensus Crate Safety (`consensus/` excluding `types/`) + +Extra scrutiny required - bugs here cause consensus failures. + +```rust +// NEVER standard arithmetic +let result = a + b; + +// ALWAYS safe math +let result = a.saturating_add(b); +// or +use safe_arith::SafeArith; +let result = a.safe_add(b)?; +``` + +Requirements: +- Use `saturating_*` or `checked_*` operations +- Zero panics - no `.unwrap()`, `.expect()`, or `array[i]` +- Deterministic behavior across all platforms + +### Error Handling + +- Return `Result` or `Option` instead of panicking +- Log errors, don't silently swallow them +- Provide context with errors + +### Async Patterns + +```rust +// NEVER block in async context +async fn handler() { + expensive_computation(); // blocks runtime +} + +// ALWAYS spawn blocking +async fn handler() { + tokio::task::spawn_blocking(|| expensive_computation()).await?; +} +``` + +### Concurrency + +- **Lock ordering**: Document lock ordering to avoid deadlocks. See [`canonical_head.rs:9-32`](beacon_node/beacon_chain/src/canonical_head.rs) for excellent example documenting three locks and safe acquisition order. +- Keep lock scopes narrow +- Seek detailed review for lock-related changes + +### Rayon Thread Pools + +Avoid using the rayon global thread pool - it causes CPU oversubscription when beacon processor has fully allocated all CPUs to workers. Use scoped rayon pools started by beacon processor for computationally intensive tasks. + +### Tracing Spans + +- Avoid spans on simple getter methods (performance overhead) +- Be cautious of span explosion with recursive functions +- Use spans per meaningful computation step, not every function +- **Never** use `span.enter()` or `span.entered()` in async tasks + +### Documentation + +- All `TODO` comments must link to a GitHub issue +- Prefer line comments (`//`) over block comments +- Keep comments concise, explain "why" not "what" + +## Logging Levels + +| Level | Use Case | +|-------|----------| +| `crit` | Lighthouse may not function - needs immediate attention | +| `error` | Moderate impact - expect user reports | +| `warn` | Unexpected but recoverable | +| `info` | High-level status - not excessive | +| `debug` | Developer events, expected errors | + +## Testing Patterns + +- **Unit tests**: Single component edge cases +- **Integration tests**: Use [`BeaconChainHarness`](beacon_node/beacon_chain/src/test_utils.rs) for end-to-end workflows +- **Sync components**: Use [`TestRig`](beacon_node/network/src/sync/tests/mod.rs) pattern with event-based testing +- **Mocking**: `mockall` for unit tests, `mockito` for HTTP APIs +- **Adapter pattern**: For testing `BeaconChain` dependent components, create adapter structs. See [`fetch_blobs/tests.rs`](beacon_node/beacon_chain/src/fetch_blobs/tests.rs) +- **Local testnet**: See `scripts/local_testnet/README.md` + +## Build Notes + +- Full builds take 5+ minutes - use large timeouts (300s+) +- Use `cargo check` for faster iteration +- MSRV documented in `Cargo.toml` + +### Cross-compilation + +- `make build-x86_64` - Cross-compile for x86_64 Linux +- `make build-aarch64` - Cross-compile for ARM64 Linux +- `make build-riscv64` - Cross-compile for RISC-V 64-bit Linux + +## Parallel Development + +For working on multiple branches simultaneously, use git worktrees: + +```bash +git worktree add -b my-feature ../lighthouse-my-feature unstable +``` + +This creates a separate working directory without needing multiple clones. To save disk space across worktrees, configure a shared target directory: + +```bash +# In .cargo/config.toml at your workspace root +[build] +target-dir = "/path/to/shared-target" +``` diff --git a/.ai/ISSUES.md b/.ai/ISSUES.md new file mode 100644 index 0000000000..ce79198b4d --- /dev/null +++ b/.ai/ISSUES.md @@ -0,0 +1,130 @@ +# GitHub Issue & PR Guidelines + +Guidelines for creating well-structured GitHub issues and PRs for Lighthouse. + +## Issue Structure + +### Start with Description + +Always begin with `## Description`: + +```markdown +## Description + +We presently prune all knowledge of non-canonical blocks once they conflict with +finalization. The pruning is not always immediate, fork choice currently prunes +once the number of nodes reaches a threshold of 256. + +It would be nice to develop a simple system for handling messages relating to +blocks that are non-canonical. +``` + +**Guidelines:** +- First paragraph: problem and brief solution +- Provide context about current behavior +- Link to related issues, PRs, or specs +- Be technical and specific + +### Steps to Resolve (when applicable) + +```markdown +## Steps to resolve + +I see two ways to fix this: a strict approach, and a pragmatic one. + +The strict approach would only check once the slot is finalized. This would have +0 false positives, but would be slower to detect missed blocks. + +The pragmatic approach might be to only process `BeaconState`s from the canonical +chain. I don't have a strong preference between approaches. +``` + +**Guidelines:** +- Don't be overly prescriptive - present options +- Mention relevant constraints +- It's okay to say "I don't have a strong preference" + +### Optional Sections + +- `## Additional Info` - Edge cases, related issues +- `## Metrics` - Performance data, observations +- `## Version` - For bug reports + +## Code References + +**Use GitHub permalinks with commit hashes** so code renders properly: + +``` +https://github.com/sigp/lighthouse/blob/261322c3e3ee/beacon_node/beacon_processor/src/lib.rs#L809 +``` + +Get commit hash: `git rev-parse unstable` + +For line ranges: `#L809-L825` + +## Writing Style + +### Be Natural and Concise +- Direct and objective +- Precise technical terminology +- Avoid AI-sounding language + +### Be Honest About Uncertainty +- Don't guess - ask questions +- Use tentative language when appropriate ("might", "I think") +- Present multiple options without picking one + +### Think About Trade-offs +- Present multiple approaches +- Discuss pros and cons +- Consider backward compatibility +- Note performance implications + +## Labels + +**Type:** `bug`, `enhancement`, `optimization`, `code-quality`, `security`, `RFC` + +**Component:** `database`, `HTTP-API`, `fork-choice`, `beacon-processor`, etc. + +**Effort:** `good first issue`, `low-hanging-fruit`, `major-task` + +## Pull Request Guidelines + +```markdown +## Description + +[What does this PR do? Why is it needed? Be concise and technical.] + +Closes #[issue-number] + +## Additional Info + +[Breaking changes, performance impacts, migration steps, etc.] +``` + +### Commit Messages + +Format: +- First line: Brief summary (imperative mood) +- Blank line +- Additional details if needed + +``` +Add custody info API for data columns + +Implements `/lighthouse/custody/info` endpoint that returns custody group +count, custodied columns, and earliest available data column slot. +``` + +## Anti-Patterns + +- Vague descriptions without details +- No code references when describing code +- Premature solutions without understanding the problem +- Making claims without validating against codebase + +## Good Examples + +- https://github.com/sigp/lighthouse/issues/6120 +- https://github.com/sigp/lighthouse/issues/4388 +- https://github.com/sigp/lighthouse/issues/8216 diff --git a/.claude/commands/issue.md b/.claude/commands/issue.md new file mode 100644 index 0000000000..85ff46fe22 --- /dev/null +++ b/.claude/commands/issue.md @@ -0,0 +1,49 @@ +# GitHub Issue Creation Task + +You are creating a GitHub issue for the Lighthouse project. + +## Required Reading + +**Before creating an issue, read `.ai/ISSUES.md`** for issue and PR writing guidelines. + +## Structure + +1. **Description** (required) + - First paragraph: problem and brief solution + - Context about current behavior + - Links to related issues, PRs, or specs + - Technical and specific + +2. **Steps to Resolve** (when applicable) + - Present options and considerations + - Don't be overly prescriptive + - Mention relevant constraints + +3. **Code References** + - Use GitHub permalinks with commit hashes + - Get hash: `git rev-parse unstable` + +## Style + +- Natural, concise, direct +- Avoid AI-sounding language +- Be honest about uncertainty +- Present trade-offs + +## Labels to Suggest + +- **Type**: bug, enhancement, optimization, code-quality +- **Component**: database, HTTP-API, fork-choice, beacon-processor +- **Effort**: good first issue, low-hanging-fruit, major-task + +## Output + +Provide the complete issue text ready to paste into GitHub. + +## After Feedback + +If the developer refines your issue/PR text or suggests a different format: + +1. **Apply their feedback** to the current issue +2. **Offer to update docs** - Ask: "Should I update `.ai/ISSUES.md` to capture this preference?" +3. **Document patterns** the team prefers that aren't yet in the guidelines diff --git a/.claude/commands/release.md b/.claude/commands/release.md new file mode 100644 index 0000000000..1694e90cc5 --- /dev/null +++ b/.claude/commands/release.md @@ -0,0 +1,85 @@ +# Release Notes Generation Task + +You are generating release notes for a new Lighthouse version. + +## Input Required + +- **Version number** (e.g., v8.1.0) +- **Base branch** (typically `stable` for previous release) +- **Release branch** (e.g., `release-v8.1`) +- **Release name** (Rick and Morty character - check existing to avoid duplicates) + +## Step 1: Gather Changes + +```bash +# Get commits between branches +git log --oneline origin/..origin/ + +# Check existing release names +gh release list --repo sigp/lighthouse --limit 50 +``` + +## Step 2: Analyze PRs + +For each PR: +1. Extract PR numbers from commit messages +2. Check for `backwards-incompat` label: + ```bash + gh pr view --repo sigp/lighthouse --json labels --jq '[.labels[].name] | join(",")' + ``` +3. Get PR details for context + +## Step 3: Categorize + +Group into sections (skip empty): +- **Breaking Changes** - schema changes, CLI changes, API changes +- **Performance Improvements** - user-noticeable optimizations +- **Validator Client Improvements** - VC-specific changes +- **Other Notable Changes** - new features, metrics +- **CLI Changes** - new/changed flags (note if BN or VC) +- **Bug Fixes** - significant user-facing fixes only + +## Step 4: Write Release Notes + +```markdown +## + +## Summary + +Lighthouse v includes . + +This is a upgrade for . + +##
+ +- **** (#<PR>): <User-facing description> + +## Update Priority + +| User Class | Beacon Node | Validator Client | +|:------------------|:------------|:-----------------| +| Staking Users | Low/Medium/High | Low/Medium/High | +| Non-Staking Users | Low/Medium/High | --- | + +## All Changes + +- <commit title> (#<PR>) + +## Binaries + +[See pre-built binaries documentation.](https://lighthouse-book.sigmaprime.io/installation_binaries.html) +``` + +## Guidelines + +- State **user impact**, not implementation details +- Avoid jargon users won't understand +- For CLI flags, mention if BN or VC +- Check PR descriptions for context + +## Step 5: Generate Announcements + +Create drafts for: +- **Email** - Formal, include priority table +- **Discord** - Tag @everyone, shorter +- **Twitter** - Single tweet, 2-3 highlights diff --git a/.claude/commands/review.md b/.claude/commands/review.md new file mode 100644 index 0000000000..7867716c79 --- /dev/null +++ b/.claude/commands/review.md @@ -0,0 +1,57 @@ +# Code Review Task + +You are reviewing code for the Lighthouse project. + +## Required Reading + +**Before reviewing, read `.ai/CODE_REVIEW.md`** for Lighthouse-specific safety requirements and review etiquette. + +## Focus Areas + +1. **Consensus Crate Safety** (if applicable) + - Safe math operations (saturating_*, checked_*) + - Zero panics + - Deterministic behavior + +2. **General Code Safety** + - No `.unwrap()` or `.expect()` at runtime + - No array indexing without bounds checks + - Proper error handling + +3. **Code Clarity** + - Clear variable names (avoid ambiguous abbreviations) + - Well-documented complex logic + - TODOs linked to GitHub issues + +4. **Error Handling** + - Errors are logged, not silently swallowed + - Edge cases are handled + - Return values are checked + +5. **Concurrency & Performance** + - Lock ordering is safe + - No blocking in async context + - Proper use of rayon thread pools + +## Output + +- Keep to 3-5 actionable comments +- Use natural, conversational language +- Provide specific line references +- Ask questions rather than making demands + +## After Review Discussion + +If the developer corrects your feedback or you learn something new: + +1. **Acknowledge and learn** - Note what you got wrong +2. **Offer to update docs** - Ask: "Should I update `.ai/CODE_REVIEW.md` with this lesson?" +3. **Format the lesson:** + ```markdown + ### Lesson: [Title] + **Issue:** [What went wrong] + **Feedback:** [What developer said] + **Learning:** [What to do differently] + ``` + +This keeps the review guidelines improving over time. diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000000..4ab3ec9333 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,10 @@ +# Lighthouse AI Assistant Guide + +See [`CLAUDE.md`](CLAUDE.md) for AI assistant guidance. + +This file exists for OpenAI Codex compatibility. Codex can read files, so refer to `CLAUDE.md` for the full documentation including: + +- Quick reference commands +- Critical rules (panics, safe math, async) +- Project structure +- Pointers to detailed guides in `.ai/` diff --git a/CLAUDE.md b/CLAUDE.md index 3e9ab169f3..441c8e4274 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,332 +1,151 @@ -# CLAUDE.md +# Lighthouse AI Assistant Guide -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. +This file provides guidance for AI assistants (Claude Code, Codex, etc.) working with Lighthouse. -## Development Commands +## Quick Reference -**Important**: Always branch from `unstable` and target `unstable` when creating pull requests. +```bash +# Build +make install # Build and install Lighthouse +cargo build --release # Standard release build -### Building and Installation +# Test (prefer targeted tests when iterating) +cargo nextest run -p <package> # Test specific package +cargo nextest run -p <package> <test> # Run individual test +make test # Full test suite (~20 min) -- `make install` - Build and install the main Lighthouse binary in release mode -- `make install-lcli` - Build and install the `lcli` utility binary -- `cargo build --release` - Standard Rust release build -- `cargo build --bin lighthouse --features "gnosis,slasher-lmdb"` - Build with specific features - -### Testing - -- `make test` - Run the full test suite in release mode (excludes EF tests, beacon_chain, slasher, network, http_api) -- `make test-release` - Run tests using nextest (faster parallel test runner) -- `make test-beacon-chain` - Run beacon chain tests for all supported forks -- `make test-slasher` - Run slasher tests with all database backend combinations -- `make test-ef` - Download and run Ethereum Foundation test vectors -- `make test-full` - Complete test suite including linting, EF tests, and execution engine tests -- `cargo nextest run -p <package_name>` - Run tests for a specific package -- `cargo nextest run -p <package_name> <test_name>` - Run individual test (preferred during development iteration) -- `FORK_NAME=electra cargo nextest run -p beacon_chain` - Run tests for specific fork - -**Note**: Full test suite takes ~20 minutes. When iterating, prefer running individual tests. - -### Linting and Code Quality - -- `make lint` - Run Clippy linter with project-specific rules -- `make lint-full` - Run comprehensive linting including tests (recommended for thorough checking) -- `make cargo-fmt` - Check code formatting with rustfmt -- `make check-benches` - Typecheck benchmark code -- `make audit` - Run security audit on dependencies - -### Cross-compilation - -- `make build-x86_64` - Cross-compile for x86_64 Linux -- `make build-aarch64` - Cross-compile for ARM64 Linux -- `make build-riscv64` - Cross-compile for RISC-V 64-bit Linux - -## Architecture Overview - -Lighthouse is a modular Ethereum consensus client with two main components: - -### Core Components - -**Beacon Node** (`beacon_node/`) - -- Main consensus client that syncs with the Ethereum network -- Contains the beacon chain state transition logic (`beacon_node/beacon_chain/`) -- Handles networking, storage, and P2P communication -- Provides HTTP API for validator clients and external tools -- Entry point: `beacon_node/src/lib.rs` - -**Validator Client** (`validator_client/`) - -- Manages validator keystores and performs validator duties -- Connects to beacon nodes via HTTP API -- Handles block proposals, attestations, and sync committee duties -- Includes slashing protection and doppelganger detection -- Entry point: `validator_client/src/lib.rs` - -### Key Subsystems - -**Consensus Types** (`consensus/types/`) - -- Core Ethereum consensus data structures (BeaconState, BeaconBlock, etc.) -- Ethereum specification implementations for different networks (mainnet, gnosis) -- SSZ encoding/decoding and state transition primitives - -**Storage** (`beacon_node/store/`) - -- Hot/cold database architecture for efficient beacon chain storage -- Supports multiple backends (LevelDB, RocksDB, REDB) -- Handles state pruning and historical data management - -**Networking** (`beacon_node/lighthouse_network/`, `beacon_node/network/`) - -- Libp2p-based P2P networking stack -- Gossipsub for message propagation -- Discovery v5 for peer discovery -- Request/response protocols for sync - -**Fork Choice** (`consensus/fork_choice/`, `consensus/proto_array/`) - -- Implements Ethereum's fork choice algorithm (proto-array) -- Manages chain reorganizations and finality - -**Execution Layer Integration** (`beacon_node/execution_layer/`) - -- Interfaces with execution clients -- Retrieves payloads from local execution layer or external block builders -- Handles payload validation and builder integration - -**Slasher** (`slasher/`) - -- Optional slashing detection service -- Supports LMDB, MDBX, and REDB database backends -- Can be enabled with `--slasher` flag - -### Utilities - -**Account Manager** (`account_manager/`) - CLI tool for managing validator accounts and keystores -**LCLI** (`lcli/`) - Lighthouse command-line utilities for debugging and testing -**Database Manager** (`database_manager/`) - Database maintenance and migration tools - -### Build System Notes - -- Uses Cargo workspace with 90+ member crates -- Supports multiple Ethereum specifications via feature flags (`gnosis`, `spec-minimal`) -- Cross-compilation support for Linux x86_64, ARM64, and RISC-V -- Multiple build profiles: `release`, `maxperf`, `reproducible` -- Feature-based compilation for different database backends and optional components - -### Network Support - -- **Mainnet**: Default production network -- **Gnosis**: Alternative network (requires `gnosis` feature) -- **Testnets**: Holesky, Sepolia via built-in network configs -- **Custom networks**: Via `--testnet-dir` flag - -### Key Configuration - -- Default data directory: `~/.lighthouse/{network}` -- Beacon node data: `~/.lighthouse/{network}/beacon` -- Validator data: `~/.lighthouse/{network}/validators` -- Configuration primarily via CLI flags and YAML files - -## Common Review Standards - -### CI/Testing Requirements - -- All checks must pass before merge -- Test coverage expected for significant changes -- Flaky tests are actively addressed and fixed -- New features often require corresponding tests -- `beacon_chain` and `http_api` tests support fork-specific testing using `FORK_NAME` env var when `beacon_chain/fork_from_env` feature is enabled - -### Code Quality Standards - -- Clippy warnings must be fixed promptly (multiple PRs show this pattern) -- Code formatting with `cargo fmt` enforced -- Must run `cargo sort` when adding dependencies - dependency order is enforced on CI -- Performance considerations for hot paths - -### Documentation and Context - -- PRs require clear descriptions of what and why -- Breaking changes need migration documentation -- API changes require documentation updates -- When CLI is updated, run `make cli-local` to generate updated help text in lighthouse book -- Comments appreciated for complex logic - -### Security and Safety - -- Careful review of consensus-critical code paths -- Error handling patterns must be comprehensive -- Input validation for external data - -## Development Patterns and Best Practices - -### Panics and Error Handling - -- **Panics should be avoided at all costs** -- Always prefer returning a `Result` or `Option` over causing a panic (e.g., prefer `array.get(1)?` over `array[1]`) -- Avoid `expect` or `unwrap` at runtime - only acceptable during startup when validating CLI flags or configurations -- If you must make assumptions about panics, use `.expect("Helpful message")` instead of `.unwrap()` and provide detailed reasoning in nearby comments -- Use proper error handling with `Result` types and graceful error propagation - -### Rayon Usage - -- Avoid using the rayon global thread pool as it results in CPU oversubscription when the beacon processor has fully allocated all CPUs to workers -- Use scoped rayon pools started by beacon processor for computational intensive tasks - -### Locks - -- Take great care to avoid deadlocks when working with fork choice locks - seek detailed review ([reference](beacon_node/beacon_chain/src/canonical_head.rs:9)) -- Keep lock scopes as narrow as possible to avoid blocking fast-responding functions like the networking stack - -### Async Patterns - -- Avoid blocking computations in async tasks -- Spawn a blocking task instead for CPU-intensive work - -### Tracing - -- Design spans carefully and avoid overuse of spans just to add context data to events -- Avoid using spans on simple getter methods as it can result in performance overhead -- Be cautious of span explosion with recursive functions -- Use spans per meaningful step or computationally critical step -- Avoid using `span.enter()` or `span.entered()` in async tasks - -### Database - -- Maintain schema continuity on `unstable` branch -- Database migrations must be backward compatible - -### Consensus Crate - -- Use safe math methods like `saturating_xxx` or `checked_xxx` -- Critical that this crate behaves deterministically and MUST not have undefined behavior - -### Testing Patterns - -- **Use appropriate test types for the right scenarios**: - - **Unit tests** for single component edge cases and isolated logic - - **Integration tests** using [`BeaconChainHarness`](beacon_node/beacon_chain/src/test_utils.rs:668) for end-to-end workflows -- **`BeaconChainHarness` guidelines**: - - Excellent for integration testing but slower than unit tests - - Prefer unit tests instead for testing edge cases of single components - - Reserve for testing component interactions and full workflows -- **Mocking strategies**: - - Use `mockall` crate for unit test mocking - - Use `mockito` for HTTP API mocking (see [`validator_test_rig`](testing/validator_test_rig/src/mock_beacon_node.rs:20) for examples) -- **Event-based testing for sync components**: - - Use [`TestRig`](beacon_node/network/src/sync/tests/mod.rs) pattern for testing sync components - - Sync components interact with the network and beacon chain via events (their public API), making event-based testing more suitable than using internal functions and mutating internal states - - Enables testing of complex state transitions and timing-sensitive scenarios -- **Testing `BeaconChain` dependent components**: - - `BeaconChain` is difficult to create for TDD - - Create intermediate adapter structs to enable easy mocking - - See [`beacon_node/beacon_chain/src/fetch_blobs/tests.rs`](beacon_node/beacon_chain/src/fetch_blobs/tests.rs) for the adapter pattern -- **Local testnet for manual/full E2E testing**: - - Use Kurtosis-based local testnet setup for comprehensive testing - - See [`scripts/local_testnet/README.md`](scripts/local_testnet/README.md) for setup instructions - -### TODOs and Comments - -- All `TODO` statements must be accompanied by a GitHub issue link -- Prefer line (`//`) comments to block comments (`/* ... */`) -- Use doc comments (`///`) before attributes for public items -- Keep documentation concise and clear - avoid verbose explanations -- Provide examples in doc comments for public APIs when helpful - -## Logging Guidelines - -Use appropriate log levels for different scenarios: - -- **`crit`**: Critical issues with major impact to Lighthouse functionality - Lighthouse may not function correctly without resolving. Needs immediate attention. -- **`error`**: Error cases that may have moderate impact to Lighthouse functionality. Expect to receive reports from users for this level. -- **`warn`**: Unexpected code paths that don't have major impact - fully recoverable. Expect user reports if excessive warning logs occur. -- **`info`**: High-level logs indicating beacon node status and block import status. Should not be used excessively. -- **`debug`**: Events lower level than info useful for developers. Can also log errors expected during normal operation that users don't need to action. - -## Code Examples - -### Safe Math in Consensus Crate - -```rust -// ❌ Avoid - could panic -let result = a + b; - -// ✅ Preferred -let result = a.saturating_add(b); -// or -use safe_arith::SafeArith; - -let result = a.safe_add(b)?; +# Lint +make lint # Run Clippy +cargo fmt --all && make lint-fix # Format and fix ``` -### Panics and Error Handling +## Before You Start + +Read the relevant guide for your task: + +| Task | Read This First | +|------|-----------------| +| **Code review** | `.ai/CODE_REVIEW.md` | +| **Creating issues/PRs** | `.ai/ISSUES.md` | +| **Development patterns** | `.ai/DEVELOPMENT.md` | + +## Critical Rules (consensus failures or crashes) + +### 1. No Panics at Runtime ```rust -// ❌ Avoid - could panic at runtime -let value = some_result.unwrap(); +// NEVER +let value = option.unwrap(); let item = array[1]; -// ✅ Preferred - proper error handling -let value = some_result.map_err(|e| CustomError::SomeVariant(e))?; +// ALWAYS +let value = option?; let item = array.get(1)?; - -// ✅ Acceptable during startup for CLI/config validation -let config_value = matches.get_one::<String>("required-flag") - .expect("Required flag must be present due to clap validation"); - -// ✅ If you must make runtime assumptions, use expect with explanation -let item = array.get(1).expect("Array always has at least 2 elements due to validation in constructor"); -// Detailed reasoning should be provided in nearby comments ``` -### TODO Format +Only acceptable during startup for CLI/config validation. + +### 2. Consensus Crate: Safe Math Only + +In `consensus/` (excluding `types/`), use saturating or checked arithmetic: ```rust -pub fn my_function(&mut self, _something: &[u8]) -> Result<String, Error> { - // TODO: Implement proper validation here - // https://github.com/sigp/lighthouse/issues/1234 -} +// NEVER +let result = a + b; + +// ALWAYS +let result = a.saturating_add(b); ``` -### Async Task Spawning for Blocking Work +## Important Rules (bugs or performance issues) + +### 3. Never Block Async ```rust -// ❌ Avoid - blocking in async context -async fn some_handler() { - let result = expensive_computation(); // blocks async runtime -} +// NEVER +async fn handler() { expensive_computation(); } -// ✅ Preferred -async fn some_handler() { - let result = tokio::task::spawn_blocking(|| { - expensive_computation() - }).await?; +// ALWAYS +async fn handler() { + tokio::task::spawn_blocking(|| expensive_computation()).await?; } ``` -### Tracing Span Usage +### 4. Lock Ordering -```rust -// ❌ Avoid - span on simple getter -#[instrument] -fn get_head_block_root(&self) -> Hash256 { - self.head_block_root -} +Document lock ordering to avoid deadlocks. See [`canonical_head.rs:9-32`](beacon_node/beacon_chain/src/canonical_head.rs) for the pattern. -// ✅ Preferred - span on meaningful operations -#[instrument(skip(self))] -async fn process_block(&self, block: Block) -> Result<(), Error> { - // meaningful computation -} +### 5. Rayon Thread Pools + +Use scoped rayon pools from beacon processor, not global pool. Global pool causes CPU oversubscription when beacon processor has allocated all CPUs. + +## Good Practices + +### 6. TODOs Need Issues + +All `TODO` comments must link to a GitHub issue. + +### 7. Clear Variable Names + +Avoid ambiguous abbreviations (`bb`, `bl`). Use `beacon_block`, `blob`. + +## Branch & PR Guidelines + +- Branch from `unstable`, target `unstable` for PRs +- Run `cargo sort` when adding dependencies +- Run `make cli-local` when updating CLI flags + +## Project Structure + +``` +beacon_node/ # Consensus client + beacon_chain/ # State transition logic + store/ # Database (hot/cold) + network/ # P2P networking + execution_layer/ # EL integration +validator_client/ # Validator duties +consensus/ + types/ # Core data structures + fork_choice/ # Proto-array ``` -## Build and Development Notes +See `.ai/DEVELOPMENT.md` for detailed architecture. -- Full builds and tests take 5+ minutes - use large timeouts (300s+) for any `cargo build`, `cargo nextest`, or `make` commands -- Use `cargo check` for faster iteration during development and always run after code changes -- Prefer targeted package tests (`cargo nextest run -p <package>`) and individual tests over full test suite when debugging specific issues -- Use `cargo fmt --all && make lint-fix` to format code and fix linting issues once a task is complete -- Always understand the broader codebase patterns before making changes -- Minimum Supported Rust Version (MSRV) is documented in `lighthouse/Cargo.toml` - ensure Rust version meets or exceeds this requirement +## Maintaining These Docs + +**These AI docs should evolve based on real interactions.** + +### After Code Reviews + +If a developer corrects your review feedback or points out something you missed: +- Ask: "Should I update `.ai/CODE_REVIEW.md` with this lesson?" +- Add to the "Common Review Patterns" or create a new "Lessons Learned" entry +- Include: what went wrong, what the feedback was, what to do differently + +### After PR/Issue Creation + +If a developer refines your PR description or issue format: +- Ask: "Should I update `.ai/ISSUES.md` to capture this?" +- Document the preferred style or format + +### After Development Work + +If you learn something about the codebase architecture or patterns: +- Ask: "Should I update `.ai/DEVELOPMENT.md` with this?" +- Add to relevant section or create new patterns + +### Format for Lessons + +```markdown +### Lesson: [Brief Title] + +**Context:** [What task were you doing?] +**Issue:** [What went wrong or was corrected?] +**Learning:** [What to do differently next time] +``` + +### When NOT to Update + +- Minor preference differences (not worth documenting) +- One-off edge cases unlikely to recur +- Already covered by existing documentation