refactor(l1): snap sync code reorganization and error handling consolidation (#5975)

## Summary

Major refactoring of the snap sync implementation (~6,500 lines across 7
files) to improve code organization, maintainability, and error
handling. This PR completes all 5 phases of the snap sync refactoring
plan.

## Motivation

The snap sync codebase had grown organically with:
- Large monolithic files (sync.rs at 1,648 lines, peer_handler.rs at
2,074 lines)
- Scattered error types across multiple modules
- Mixed concerns (server/client code, protocol messages, sync
orchestration)
- Inconsistent naming conventions in healing modules

This refactoring addresses these issues through a structured, phased
approach.

## Changes by Phase

### Phase 1: Foundation ✅
- Created `snap/` module directory structure
- Moved server code from `snap.rs` to `snap/server.rs`
- Created `snap/constants.rs` with documented protocol constants
- Moved snap server tests to root test crate
(`test/tests/p2p/snap_server_tests.rs`) to avoid `clippy::unwrap_used`
lint denial inherited from the `ethrex-p2p` crate

### Phase 2: Protocol Layer ✅
- Split `rlpx/snap.rs` into `rlpx/snap/` directory:
  - `messages.rs` - Snap protocol message type definitions
  - `codec.rs` - RLPxMessage encoding/decoding implementations
  - `mod.rs` - Module re-exports

### Phase 3: Healing Unification ✅
- Created `sync/healing/` directory with unified structure:
- `types.rs` - Shared healing types (HealingQueueEntry,
StateHealingQueue)
  - `state.rs` - State trie healing (~420 lines)
  - `storage.rs` - Storage trie healing (~530 lines)
- Improved naming conventions:
  - `MembatchEntryValue` → `HealingQueueEntry`
  - `MembatchEntry` → `StorageHealingQueueEntry`
  - `Membatch` → `StorageHealingQueue`
  - `children_not_in_storage_count` → `missing_children_count`

### Phase 4: Sync Orchestration ✅
- Split `sync.rs` (1,648 lines) into focused modules:
  - `sync/full.rs` (~260 lines) - Full sync implementation
  - `sync/snap_sync.rs` (~1,100 lines) - Snap sync implementation
  - `sync/mod.rs` (~285 lines) - Syncer struct and orchestration
- Extracted snap client methods from `peer_handler.rs` to
`snap/client.rs`:
- `request_account_range`, `request_bytecodes`, `request_storage_ranges`
  - `request_state_trienodes`, `request_storage_trienodes`
  - Reduced `peer_handler.rs` from 2,060 to 670 lines (~68% reduction)

### Phase 5: Error Handling ✅
- Created unified `snap/error.rs` with `SnapError` enum:
  - `Store` - Storage layer errors
  - `Protocol` - Connection/protocol errors  
  - `Trie` - Trie operation errors
  - `RlpDecode` - RLP decoding errors
  - `PeerTable` - Peer table errors
  - `BadRequest` - Invalid request validation
  - `ValidationError` - Response validation failures
  - `NoTasks`, `NoAccountHashes`, `NoAccountStorages`, `NoStorageRoots`
  - `InternalError`, `FileSystem`, `SnapshotDir`, `TaskPanic`
  - `InvalidData`, `InvalidHash`
- Removed redundant error types: `SnapClientError`,
`RequestStateTrieNodesError`
- Added `From<SnapError>` for `PeerConnectionError` for error
propagation
- Kept `RequestStorageTrieNodesError` struct for request ID tracking

## Bug Fixes

Fixed several potential panics and edge cases in `snap/client.rs`:

- **Empty vector indexing**: Replaced direct `[0]` indexing with safe
`.get(0).ok_or()` or `.first().unwrap_or()` to prevent panics on empty
vectors
- **Zero chunk_size in `request_bytecodes`**: Added early return for
empty input and adjusted `chunk_count` to not exceed input length
- **Zero chunk_size in `request_account_range`**: Ensured `chunk_count`
doesn't exceed the hash range to avoid zero-sized chunks
- **Typo fix**: "Trie to get" → "Tried to get" in error message

## New Module Structure

```
crates/networking/p2p/
├── snap/
│   ├── mod.rs          # Re-exports
│   ├── server.rs       # Server-side request processing
│   ├── client.rs       # Client-side request methods (~1,439 lines)
│   ├── constants.rs    # Protocol constants
│   └── error.rs        # Unified SnapError type
├── rlpx/snap/
│   ├── mod.rs          # Re-exports
│   ├── messages.rs     # Message struct definitions
│   └── codec.rs        # RLPxMessage implementations
├── sync/
│   ├── mod.rs          # Syncer and orchestration (~285 lines)
│   ├── full.rs         # Full sync (~260 lines)
│   ├── snap_sync.rs    # Snap sync (~1,100 lines)
│   └── healing/
│       ├── mod.rs      # Re-exports
│       ├── types.rs    # Shared healing types
│       ├── state.rs    # State healing (~420 lines)
│       └── storage.rs  # Storage healing (~530 lines)
└── peer_handler.rs     # ETH protocol methods (~670 lines)

test/tests/p2p/
└── snap_server_tests.rs  # Snap server tests (moved from p2p crate)
```

## Testing

- ✅ `cargo check -p ethrex-p2p` - Compilation passes
- ✅ `cargo clippy -p ethrex-p2p` - No warnings
- ✅ `cargo test -p ethrex-p2p` - All 36 unit tests pass
- ✅ `cargo test -p ethrex-test snap_server` - All 12 snap server tests
pass (moved to root test crate)

## Checklist

- [ ] Updated `STORE_SCHEMA_VERSION` (crates/storage/lib.rs) if the PR
includes breaking changes to the `Store` requiring a re-sync.

---

## Refactoring Plan Details

<details>
<summary>Click to expand the full refactoring plan</summary>

### Overview

The Snap Sync implementation spans ~6,500 lines across 7 files. This
plan provides a structured approach to simplify and improve the code.

### Files Involved

#### Original Structure
| File | Lines | Purpose |
|------|-------|---------|
| `crates/networking/p2p/snap.rs` | 1,008 | Server-side request
processing (90% tests) |
| `crates/networking/p2p/rlpx/snap.rs` | 389 | Protocol message
definitions |
| `crates/networking/p2p/sync.rs` | 1,648 | Main sync orchestration |
| `crates/networking/p2p/sync/state_healing.rs` | 471 | State trie
healing |
| `crates/networking/p2p/sync/storage_healing.rs` | 718 | Storage
healing |
| `crates/networking/p2p/sync/code_collector.rs` | 102 | Bytecode
collection |
| `crates/networking/p2p/peer_handler.rs` | 2,074 | Client-side snap
requests (~800 lines snap-related) |

#### New Structure (After Phases 1-5)
| File | Purpose |
|------|---------|
| `crates/networking/p2p/snap/mod.rs` | Snap module re-exports |
| `crates/networking/p2p/snap/server.rs` | Server-side request
processing |
| `crates/networking/p2p/snap/client.rs` | Client-side snap request
methods (~1,439 lines) |
| `crates/networking/p2p/snap/constants.rs` | Centralized protocol
constants |
| `crates/networking/p2p/rlpx/snap/mod.rs` | Protocol message re-exports
|
| `crates/networking/p2p/rlpx/snap/messages.rs` | Message struct
definitions |
| `crates/networking/p2p/rlpx/snap/codec.rs` | RLPxMessage
implementations |
| `crates/networking/p2p/sync/mod.rs` | Sync orchestration (~285 lines)
|
| `crates/networking/p2p/sync/full.rs` | Full sync implementation (~260
lines) |
| `crates/networking/p2p/sync/snap_sync.rs` | Snap sync implementation
(~1,100 lines) |
| `crates/networking/p2p/sync/healing/mod.rs` | Healing module
re-exports |
| `crates/networking/p2p/sync/healing/types.rs` | Shared healing types |
| `crates/networking/p2p/sync/healing/state.rs` | State healing (~420
lines) |
| `crates/networking/p2p/sync/healing/storage.rs` | Storage healing
(~530 lines) |
| `crates/networking/p2p/peer_handler.rs` | ETH protocol requests (~670
lines) |
| `test/tests/p2p/snap_server_tests.rs` | Snap server tests (in root
test crate) |

### Implementation Order (Dependencies)

```
Phase 1.1-1.2 (snap module)
    ↓
Phase 1.3 (constants) ──→ Phase 2.1-2.3 (rlpx reorganization)
    ↓
Phase 1.4-1.5 (tests, imports)
    ↓
Phase 3.1-3.2 (pending_nodes)
    ↓
Phase 3.3-3.5 (healing unification)
    ↓
Phase 4.1-4.3 (sync split)
    ↓
Phase 4.4-4.5 (snap client extraction)
    ↓
Phase 5.1-5.3 (error consolidation)
```

### Risk Mitigation

| Phase | Risk | Mitigation |
|-------|------|------------|
| 1. Foundation | Low | Simple reorganization, APIs unchanged |
| 2. Protocol | Medium | Extensive hive testing |
| 3. Healing | Medium-High | Incremental migration, keep old files until
verified |
| 4. Sync orchestration | High | Feature flags, integration tests,
gradual extraction |
| 5. Error handling | Medium | Keep old errors, wrap in new type
initially |

### Decisions Made
- **No RLPxMessage macro**: Implementations vary too much (e.g.,
`GetStorageRanges` special hash handling)
- **Backward-compatible re-exports**: Constants re-exported from
`peer_handler.rs` to avoid breaking changes
- **Incremental approach**: Each phase builds on previous, allowing
verification at each step

### Key Considerations
- The `accounts_by_root_hash` structure in sync is unbounded - consider
adding limits
- Tests should cover edge cases for hash boundary handling in
`GetStorageRanges`
- Healing processes share similar patterns but have distinct algorithms
- trait unification should preserve this

</details>

Author: pablodeymo

Cargo.lock

@@ -74,7 +74,7 @@ pub(crate) mod metrics;
 pub mod network;
 pub mod peer_handler;
 pub mod rlpx;
-pub(crate) mod snap;
+pub mod snap;
 pub mod sync;
 pub mod sync_manager;
 pub mod tx_broadcaster;