Improve code quality and documentation for better maintainability (#12)

Author: rezbera

Cargo.lock

@@ -31,6 +31,7 @@ eyre = "0.6"
 jsonrpsee-core = "0.25.1"
 log = "0.4.27"
 serde = { version = "1.0", features = ["derive"], default-features = false }
+thiserror = "2.0"
 
 
 [patch.crates-io]

Cargo.toml

@@ -52,11 +52,44 @@ The binary will be at `target/release/bera-reth`.
 
 ---
 
-## ▶️ Running Locally with BeaconKit
+## ▶️ Running with BeaconKit (Local Development)
 
-1. Run `make start` from **your Beacon-Kit repository**. 
-2. Save the path to your BeaconKit repository in the `BEACON_KIT` envar, e.g. `export BEACON_KIT=/Users/rezbera/Code/beacon-kit`
-3. Run `make start-bera-reth-local` from **this repository**
+For local development and testing, you can use the provided script to test BeaconKit integration:
+
+```bash
+# Basic usage (tests progression to block 10 with 120s timeout)
+BEACON_KIT_PATH=/path/to/beacon-kit ./scripts/test-block-progression.sh
+
+# Custom configuration
+BEACON_KIT_PATH=/path/to/beacon-kit TARGET_BLOCK=5 TIMEOUT=180 ./scripts/test-block-progression.sh
+```
+
+### Prerequisites
+
+- Local BeaconKit repository cloned and built
+- Set `BEACON_KIT_PATH` to your BeaconKit directory
+
+### Environment Variables
+
+- `BEACON_KIT_PATH`: Path to your BeaconKit repository (required)
+- `TARGET_BLOCK`: Target block number to reach (default: `10`)
+- `TIMEOUT`: Maximum time to wait in seconds (default: `120`)
+
+### What the script does
+
+1. Cleans up any existing data directories
+2. Starts BeaconKit locally with `[BEACONKIT]` log prefixes
+3. Starts bera-reth with `[RETH]` log prefixes
+4. Monitors block progression via JSON-RPC calls
+5. Reports success when target block is reached
+6. Automatically cleans up all processes on exit or Ctrl+C
+
+### Manual Setup (Alternative)
+
+If you prefer to run the components manually:
+
+1. Run `make start` from **your BeaconKit repository**
+2. Run `BEACON_KIT=/path/to/beacon-kit make start-bera-reth-local` from **this repository**
 
 ---
 
@@ -81,6 +114,19 @@ cargo audit
 cargo udeps --all-features --locked
 ```
 
+---
+
+## 📚 Documentation
+
+View the comprehensive code documentation locally:
+
+```bash
+# Build and open documentation in your browser
+cargo doc --open --no-deps --document-private-items
+```
+
+This will generate and open detailed API documentation including all modules, types, and examples.
+
 ## 📜 License
 
 Licensed under the Apache-2.0 License. See [LICENSE](LICENSE) for details.

README.md

@@ -0,0 +1,99 @@
+#!/bin/bash
+# Test BeaconKit + bera-reth block progression
+set -ex
+
+BEACON_KIT_PATH="${BEACON_KIT_PATH:-../beacon-kit}"
+TARGET_BLOCK="${TARGET_BLOCK:-10}"
+TIMEOUT="${TIMEOUT:-120}"
+
+[ ! -d "$BEACON_KIT_PATH" ] && { echo "ERROR: Set BEACON_KIT_PATH"; exit 1; }
+
+# Check if required ports are available
+for port in 8545 8551 30303 3500; do
+    if lsof -i :$port >/dev/null 2>&1; then
+        echo "ERROR: Port $port is in use"
+        exit 1
+    fi
+done
+
+cleanup() { 
+    echo "Cleaning up processes..."
+    [ -n "$BEACON_PID" ] && kill $BEACON_PID 2>/dev/null || true
+    [ -n "$RETH_PID" ] && kill $RETH_PID 2>/dev/null || true
+    pkill -f "beacond\|bera-reth" 2>/dev/null || true
+    pkill -f "make start" 2>/dev/null || true
+    pkill -f "make start-bera-reth-local" 2>/dev/null || true
+    jobs -p | xargs -r kill 2>/dev/null || true
+}
+trap cleanup EXIT INT TERM
+
+get_block() {
+    result=$(curl -s -X POST -H "Content-Type: application/json" \
+         --data '{"jsonrpc":"2.0","method":"eth_blockNumber","params":[],"id":1}' \
+         http://localhost:8545 2>/dev/null | \
+    grep -o '"result":"[^"]*"' | cut -d'"' -f4 2>/dev/null)
+    if [ -n "$result" ]; then
+        printf "%d\n" "$result" 2>/dev/null || echo "0"
+    else
+        echo "0"
+    fi
+}
+
+echo "Testing block progression to $TARGET_BLOCK (timeout: ${TIMEOUT}s)"
+
+# Clean directories
+rm -rf /.tmp/beacond ~/.bera-reth 2>/dev/null || true
+
+# Start BeaconKit with timeout protection
+echo "Starting BeaconKit..."
+cd "$BEACON_KIT_PATH"
+bash -c 'echo "y" | make start' 2>&1 | sed 's/^/[BEACONKIT] /' &
+BEACON_PID=$!
+
+# Wait for BeaconKit to initialize with timeout
+WAIT_TIME=0
+while [ $WAIT_TIME -lt 10 ]; do
+    if [ -f "$BEACON_KIT_PATH/.tmp/beacond/eth-genesis.json" ]; then
+        echo "BeaconKit initialized successfully"
+        break
+    fi
+    sleep 2
+    WAIT_TIME=$((WAIT_TIME + 2))
+done
+
+# Verify genesis file exists
+[ ! -f "$BEACON_KIT_PATH/.tmp/beacond/eth-genesis.json" ] && { 
+    echo "ERROR: Genesis file not found after ${WAIT_TIME}s"; 
+    kill $BEACON_PID 2>/dev/null || true
+    exit 1; 
+}
+
+# Start bera-reth
+echo "Starting bera-reth..."
+cd - >/dev/null
+BEACON_KIT="$BEACON_KIT_PATH" make start-bera-reth-local 2>&1 | sed 's/^/[RETH] /' &
+RETH_PID=$!
+sleep 10
+
+# Monitor block progression
+start_time=$(date +%s)
+prev_block=0
+
+while [ $(($(date +%s) - start_time)) -lt $TIMEOUT ]; do
+    current_block=$(get_block)
+    
+    if [ "$current_block" != "0" ] && [ "$current_block" -gt "$prev_block" ]; then
+        echo "Block: $prev_block -> $current_block"
+        prev_block=$current_block
+        
+        [ "$current_block" -ge "$TARGET_BLOCK" ] && {
+            echo "SUCCESS: Reached block $current_block in $(($(date +%s) - start_time))s"
+            exit 0
+        }
+    fi
+    
+    sleep 3
+done
+
+echo "TIMEOUT: Only reached block $prev_block"
+exit 1

scripts/test-block-progression.sh

@@ -1,4 +1,4 @@
-//! Berachain chain spec
+//! Berachain chain specification with Ethereum hardforks plus Prague1 minimum base fee
 
 use crate::{
     genesis::BerachainGenesisConfig,
@@ -22,9 +22,16 @@ use reth_ethereum_cli::chainspec::SUPPORTED_CHAINS;
 use reth_evm::eth::spec::EthExecutorSpec;
 use std::{fmt::Display, sync::Arc};
 
-/// Berachain chain spec
+/// Minimum base fee enforced after Prague1 hardfork (1 gwei)
+const PRAGUE1_MIN_BASE_FEE_WEI: u64 = 1_000_000_000;
+
+/// Default minimum base fee when Prague1 is not active.
+const DEFAULT_MIN_BASE_FEE_WEI: u64 = 0;
+
+/// Berachain chain specification wrapping Reth's ChainSpec with Prague1 hardfork
 #[derive(Debug, Clone, Into, Constructor, PartialEq, Eq, Default)]
 pub struct BerachainChainSpec {
+    /// The underlying Reth chain specification
     inner: ChainSpec,
 }
 impl EthChainSpec for BerachainChainSpec {
@@ -90,8 +97,11 @@ impl EthChainSpec for BerachainChainSpec {
         // Note that we use this parent block timestamp to determine whether Prague 1 is active.
         // This means that we technically start the base_fee enforcement the block after the fork
         // block. This is a conscious decision to minimize fork diffs across execution clients.
-        let min_base_fee =
-            if self.is_prague1_active_at_timestamp(parent.timestamp()) { 1_000_000_000 } else { 0 };
+        let min_base_fee = if self.is_prague1_active_at_timestamp(parent.timestamp()) {
+            PRAGUE1_MIN_BASE_FEE_WEI
+        } else {
+            DEFAULT_MIN_BASE_FEE_WEI
+        };
 
         raw.max(min_base_fee)
     }
@@ -137,7 +147,7 @@ impl EthExecutorSpec for BerachainChainSpec {
     }
 }
 
-/// Berachain chain specification parser.
+/// Parser for Berachain chain specifications
 #[derive(Debug, Clone, Default)]
 #[non_exhaustive]
 pub struct BerachainChainSpecParser;
@@ -201,9 +211,12 @@ impl From<Genesis> for BerachainChainSpec {
             };
 
         // Time-based hardforks
+        // For the From implementation, we use a default config if parsing fails
+        // This maintains backward compatibility while preventing panics
         let berachain_genesis_config =
             BerachainGenesisConfig::try_from(&genesis.config.extra_fields).unwrap_or_else(|e| {
-                panic!("failed to parse berachain genesis config from genesis file: {e}")
+                tracing::warn!("Failed to parse berachain genesis config, using defaults: {}", e);
+                BerachainGenesisConfig::default()
             });
 
         let time_hardfork_opts = [

src/chainspec/mod.rs

@@ -1,29 +1,92 @@
+//! Berachain genesis configuration parsing and validation
+
 use jsonrpsee_core::__reexports::serde_json;
 use reth::rpc::types::serde_helpers::OtherFields;
-use serde::{Deserialize, Serialize, de::Error};
+use serde::{Deserialize, Serialize};
+use thiserror::Error;
+
+/// Errors for Berachain genesis configuration parsing
+#[derive(Debug, Error)]
+pub enum BerachainConfigError {
+    /// The required 'berachain' field is missing from the genesis configuration
+    #[error("Missing required 'berachain' field in genesis configuration")]
+    MissingBerachainField,
+
+    /// Invalid configuration format or values
+    #[error("Invalid berachain configuration: {0}")]
+    InvalidConfig(#[from] serde_json::Error),
+
+    /// Base fee change denominator cannot be zero as it would cause division by zero
+    #[error("Base fee change denominator cannot be zero")]
+    InvalidDenominator,
 
+    /// Fork activation time is invalid (e.g., in the past for future forks)
+    #[error("Invalid fork activation time: {0}")]
+    InvalidActivationTime(u64),
+}
+
+/// Configuration for a Berachain hardfork activation
 #[derive(Debug, Clone, Copy, Eq, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct BerachainForkConfig {
+    /// Unix timestamp when this hardfork activates
     pub time: u64,
+    /// Denominator for base fee change calculations (must be > 0)
     pub base_fee_change_denominator: u128,
+    /// Minimum base fee in wei enforced after activation
     pub minimum_base_fee_wei: u64,
 }
 
+/// Complete Berachain genesis configuration from JSON "berachain" field
 #[derive(Debug, Clone, Copy, Eq, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct BerachainGenesisConfig {
+    /// Configuration for the Prague1 hardfork, which introduces minimum base fee enforcement
     pub prague1: BerachainForkConfig,
 }
 
+impl Default for BerachainGenesisConfig {
+    /// Default config with Prague1 activated immediately at genesis
+    fn default() -> Self {
+        Self {
+            prague1: BerachainForkConfig {
+                time: 0,                             // Activate immediately at genesis
+                base_fee_change_denominator: 48,     // Berachain standard value
+                minimum_base_fee_wei: 1_000_000_000, // 1 gwei
+            },
+        }
+    }
+}
+
+impl BerachainForkConfig {
+    /// Creates validated config. Returns error if denominator is 0.
+    pub fn new(
+        time: u64,
+        base_fee_change_denominator: u128,
+        minimum_base_fee_wei: u64,
+    ) -> Result<Self, BerachainConfigError> {
+        if base_fee_change_denominator == 0 {
+            return Err(BerachainConfigError::InvalidDenominator);
+        }
+        Ok(Self { time, base_fee_change_denominator, minimum_base_fee_wei })
+    }
+}
+
 impl TryFrom<&OtherFields> for BerachainGenesisConfig {
-    type Error = serde_json::Error;
+    type Error = BerachainConfigError;
 
+    /// Parse BerachainGenesisConfig from genesis "berachain" field
     fn try_from(others: &OtherFields) -> Result<Self, Self::Error> {
         match others.get_deserialized::<Self>("berachain") {
-            Some(Ok(cfg)) => Ok(cfg),
-            Some(Err(e)) => Err(e), // propagate the real serde error inside
-            None => Err(Error::missing_field("berachain")),
+            Some(Ok(cfg)) => {
+                // Validate the parsed configuration
+                if cfg.prague1.base_fee_change_denominator == 0 {
+                    return Err(BerachainConfigError::InvalidDenominator);
+                }
+                Ok(cfg)
+            }
+            Some(Err(e)) => Err(BerachainConfigError::InvalidConfig(e)),
+            None => Err(BerachainConfigError::MissingBerachainField),
         }
     }
 }
@@ -45,7 +108,9 @@ mod tests {
         let other_fields = OtherFields::try_from(v).expect("must be a valid genesis config");
         let res = BerachainGenesisConfig::try_from(&other_fields);
         assert!(
-            res.expect_err("must be an error").to_string().contains("missing field `berachain`")
+            res.expect_err("must be an error")
+                .to_string()
+                .contains("Missing required 'berachain' field")
         );
     }
 

src/genesis/mod.rs

@@ -1,17 +1,21 @@
+//! Berachain hardfork definitions for use alongside Ethereum hardforks
+
 use reth::chainspec::{EthereumHardforks, ForkCondition, hardfork};
 
 hardfork!(
-    /// The name of a berachain hardfork.
-    ///
-    /// When building a list of hardforks for a chain, it's still expected to mix with [`EthereumHardfork`].
+    /// Berachain hardforks to be mixed with [`EthereumHardfork`]
     BerachainHardfork {
-        /// Berachain `Prague1` hardfork
+        /// Prague1 hardfork: Enforces 1 gwei minimum base fee
         Prague1,
     }
 );
 
+/// Trait for querying Berachain hardfork activation status
 pub trait BerachainHardforks: EthereumHardforks {
+    /// Returns activation condition for a Berachain hardfork
     fn berachain_fork_activation(&self, fork: BerachainHardfork) -> ForkCondition;
+
+    /// Checks if Prague1 hardfork is active at given timestamp
     fn is_prague1_active_at_timestamp(&self, timestamp: u64) -> bool {
         self.berachain_fork_activation(BerachainHardfork::Prague1).active_at_timestamp(timestamp)
     }

src/hardforks/mod.rs

@@ -1,3 +1,7 @@
+//! Bera-Reth: Ethereum execution client for Berachain
+//!
+//! Built on Reth SDK with Ethereum compatibility plus Prague1 hardfork for minimum base fee.
+
 pub mod chainspec;
 pub mod genesis;
 pub mod hardforks;