Skip to content

Address PR review comments: documentation consistency and code improvements #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Steake merged 2 commits into from
Nov 27, 2025
Merged

Address PR review comments: documentation consistency and code improvements #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de89ec8
Initial plan
Copilot Nov 27, 2025
edb0c5b
Address PR review comments: documentation fixes and code improvements
Copilot Nov 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/bitcell-node/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ pub struct NodeConfig {
pub enable_dht: bool,
pub bootstrap_nodes: Vec<String>,
pub key_seed: Option<String>,
/// Block production interval in seconds.
/// Defaults to 10 seconds for testing. Use 600 (10 minutes) for production.
pub block_time_secs: u64,
}

#[derive(Debug, Clone, Serialize, Deserialize)]
Expand All @@ -29,6 +32,7 @@ impl Default for NodeConfig {
enable_dht: false, // Disabled by default for backwards compatibility
bootstrap_nodes: vec![],
key_seed: None,
block_time_secs: 10, // Default to 10 seconds for testing
}
}
}
7 changes: 5 additions & 2 deletions crates/bitcell-node/src/network.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ use tokio::net::{TcpListener, TcpStream};
use tokio::io::{AsyncReadExt, AsyncWriteExt};
use serde::{Serialize, Deserialize};

/// Maximum message size limit (10MB) to prevent memory exhaustion attacks
const MAX_MESSAGE_SIZE: usize = 10_000_000;

/// Network message types
#[derive(Debug, Clone, Serialize, Deserialize)]
pub enum NetworkMessage {
Expand Down Expand Up @@ -377,7 +380,7 @@ impl NetworkManager {
.map_err(|e| format!("Read error: {}", e))?;
let len = u32::from_be_bytes(len_bytes) as usize;

if len > 10_000_000 { // 10MB safety limit
if len > MAX_MESSAGE_SIZE {
return Err("Message too large".into());
}

Expand All @@ -398,7 +401,7 @@ impl NetworkManager {
.map_err(|e| format!("Read error: {}", e))?;
let len = u32::from_be_bytes(len_bytes) as usize;

if len > 10_000_000 { // 10MB safety limit
if len > MAX_MESSAGE_SIZE {
return Err("Message too large".into());
}

Expand Down
6 changes: 2 additions & 4 deletions crates/bitcell-node/src/validator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ use std::sync::Arc;
use std::time::Duration;
use tokio::time;

/// Block production interval (10 seconds for testing, TODO: make this 10 minutes in production)
const BLOCK_TIME_SECS: u64 = 10;

/// Max transactions per block
const MAX_TXS_PER_BLOCK: usize = 1000;

Expand Down Expand Up @@ -167,9 +164,10 @@ impl ValidatorNode {
let secret_key = self.secret_key.clone();
let tournament_manager = self.tournament_manager.clone();
let network = self.network.clone();
let block_time_secs = self.config.block_time_secs;

tokio::spawn(async move {
let mut interval = time::interval(Duration::from_secs(BLOCK_TIME_SECS));
let mut interval = time::interval(Duration::from_secs(block_time_secs));
let mut next_height = 1u64;

loop {
Expand Down
32 changes: 24 additions & 8 deletions crates/bitcell-state/src/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,19 @@ impl StorageManager {
}

/// Prune old blocks (keep last N blocks)
///
/// # TODO: Production Implementation
/// This is a simplified implementation for development. A production version should:
/// - Use iterators for efficient range deletion
/// - Delete associated transactions and state roots
/// - Handle edge cases (e.g., concurrent reads during pruning)
/// - Optionally archive pruned blocks to cold storage
///
/// # Arguments
/// * `keep_last` - Number of recent blocks to retain
///
/// # Returns
/// * `Ok(())` on success, or error message on failure
pub fn prune_old_blocks(&self, keep_last: u64) -> Result<(), String> {
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newly implemented prune_old_blocks function lacks test coverage. Consider adding a test case that verifies the deletion of old blocks and headers while preserving the most recent blocks, similar to the existing test patterns in the same file (e.g., test_store_and_retrieve_header).

Copilot uses AI. Check for mistakes.
let latest = self.get_latest_height()?.unwrap_or(0);
if latest <= keep_last {
Expand All @@ -170,17 +183,20 @@ impl StorageManager {

let prune_until = latest - keep_last;

// Verify blocks column family exists
self.db.cf_handle(CF_BLOCKS)
// Get column family handles
let cf_blocks = self.db.cf_handle(CF_BLOCKS)
.ok_or_else(|| "Blocks column family not found".to_string())?;
let cf_headers = self.db.cf_handle(CF_HEADERS)
.ok_or_else(|| "Headers column family not found".to_string())?;

// This is a simplified version - in production would iterate and delete
// Iterate and delete blocks and headers for heights less than prune_until
for height in 0..prune_until {
if let Some(header_data) = self.get_header_by_height(height)? {
// Extract hash and delete block
// (Simplified - would need proper header deserialization)
let _ = header_data;
}
// Delete block by height
self.db.delete_cf(cf_blocks, height.to_be_bytes())
.map_err(|e| format!("Failed to delete block at height {}: {}", height, e))?;
// Delete header by height
self.db.delete_cf(cf_headers, height.to_be_bytes())
.map_err(|e| format!("Failed to delete header at height {}: {}", height, e))?;
}

Ok(())
Expand Down
12 changes: 12 additions & 0 deletions crates/bitcell-zkp/src/battle_constraints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ use ark_r1cs_std::bits::ToBitsGadget;
use ark_relations::r1cs::{ConstraintSynthesizer, ConstraintSystemRef, SynthesisError};

/// Size of the CA grid (must be power of 2 for efficient constraints)
///
/// # Test vs Production Configuration
/// - **Test values**: `GRID_SIZE = 64`, `BATTLE_STEPS = 10`
/// - Used for unit tests and development to enable fast proof generation
/// - Suitable for CI/CD pipelines and local testing
/// - **Production values**: `GRID_SIZE = 1024`, `BATTLE_STEPS = 1000`
/// - Used for mainnet deployment with full-size tournament battles
/// - Requires trusted setup ceremony and optimized proving infrastructure
///
/// To switch between configurations, adjust these constants before compilation.
/// For production deployment, ensure sufficient hardware for proof generation
/// (recommended: 64GB+ RAM, GPU acceleration for proving).
pub const GRID_SIZE: usize = 64; // Reduced from 1024 for practical circuit size
pub const BATTLE_STEPS: usize = 10; // Reduced from 1000 for practical proving time

Expand Down
86 changes: 43 additions & 43 deletions crates/bitcell-zkvm/src/interpreter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,92 +116,92 @@ impl Interpreter {

match inst.opcode {
OpCode::Add => {
let a = self.get_register(inst.rs1);
let b = self.get_register(inst.rs2());
self.set_register(inst.rd, a.wrapping_add(b));
let lhs = self.get_register(inst.rs1);
let rhs = self.get_register(inst.rs2());
self.set_register(inst.rd, lhs.wrapping_add(rhs));
self.pc += 1;
}
OpCode::Sub => {
let a = self.get_register(inst.rs1);
let b = self.get_register(inst.rs2());
self.set_register(inst.rd, a.wrapping_sub(b));
let lhs = self.get_register(inst.rs1);
let rhs = self.get_register(inst.rs2());
self.set_register(inst.rd, lhs.wrapping_sub(rhs));
self.pc += 1;
}
OpCode::Mul => {
let a = self.get_register(inst.rs1);
let b = self.get_register(inst.rs2());
self.set_register(inst.rd, a.wrapping_mul(b));
let lhs = self.get_register(inst.rs1);
let rhs = self.get_register(inst.rs2());
self.set_register(inst.rd, lhs.wrapping_mul(rhs));
self.pc += 1;
}
OpCode::Div => {
let a = self.get_register(inst.rs1);
let b = self.get_register(inst.rs2());
if b == 0 {
let lhs = self.get_register(inst.rs1);
let rhs = self.get_register(inst.rs2());
if rhs == 0 {
return Err(InterpreterError::DivisionByZero);
}
self.set_register(inst.rd, a / b);
self.set_register(inst.rd, lhs / rhs);
self.pc += 1;
}
OpCode::Mod => {
let a = self.get_register(inst.rs1);
let b = self.get_register(inst.rs2());
if b == 0 {
let lhs = self.get_register(inst.rs1);
let rhs = self.get_register(inst.rs2());
if rhs == 0 {
return Err(InterpreterError::DivisionByZero);
}
self.set_register(inst.rd, a % b);
self.set_register(inst.rd, lhs % rhs);
self.pc += 1;
}
OpCode::And => {
let a = self.get_register(inst.rs1);
let b = self.get_register(inst.rs2());
self.set_register(inst.rd, a & b);
let lhs = self.get_register(inst.rs1);
let rhs = self.get_register(inst.rs2());
self.set_register(inst.rd, lhs & rhs);
self.pc += 1;
}
OpCode::Or => {
let a = self.get_register(inst.rs1);
let b = self.get_register(inst.rs2());
self.set_register(inst.rd, a | b);
let lhs = self.get_register(inst.rs1);
let rhs = self.get_register(inst.rs2());
self.set_register(inst.rd, lhs | rhs);
self.pc += 1;
}
OpCode::Xor => {
let a = self.get_register(inst.rs1);
let b = self.get_register(inst.rs2());
self.set_register(inst.rd, a ^ b);
let lhs = self.get_register(inst.rs1);
let rhs = self.get_register(inst.rs2());
self.set_register(inst.rd, lhs ^ rhs);
self.pc += 1;
}
OpCode::Not => {
let a = self.get_register(inst.rs1);
self.set_register(inst.rd, !a);
let lhs = self.get_register(inst.rs1);
self.set_register(inst.rd, !lhs);
self.pc += 1;
}
OpCode::Eq => {
let a = self.get_register(inst.rs1);
let b = self.get_register(inst.rs2());
self.set_register(inst.rd, if a == b { 1 } else { 0 });
let lhs = self.get_register(inst.rs1);
let rhs = self.get_register(inst.rs2());
self.set_register(inst.rd, if lhs == rhs { 1 } else { 0 });
self.pc += 1;
}
OpCode::Lt => {
let a = self.get_register(inst.rs1);
let b = self.get_register(inst.rs2());
self.set_register(inst.rd, if a < b { 1 } else { 0 });
let lhs = self.get_register(inst.rs1);
let rhs = self.get_register(inst.rs2());
self.set_register(inst.rd, if lhs < rhs { 1 } else { 0 });
self.pc += 1;
}
OpCode::Gt => {
let a = self.get_register(inst.rs1);
let b = self.get_register(inst.rs2());
self.set_register(inst.rd, if a > b { 1 } else { 0 });
let lhs = self.get_register(inst.rs1);
let rhs = self.get_register(inst.rs2());
self.set_register(inst.rd, if lhs > rhs { 1 } else { 0 });
self.pc += 1;
}
OpCode::Le => {
let a = self.get_register(inst.rs1);
let b = self.get_register(inst.rs2());
self.set_register(inst.rd, if a <= b { 1 } else { 0 });
let lhs = self.get_register(inst.rs1);
let rhs = self.get_register(inst.rs2());
self.set_register(inst.rd, if lhs <= rhs { 1 } else { 0 });
self.pc += 1;
}
OpCode::Ge => {
let a = self.get_register(inst.rs1);
let b = self.get_register(inst.rs2());
self.set_register(inst.rd, if a >= b { 1 } else { 0 });
let lhs = self.get_register(inst.rs1);
let rhs = self.get_register(inst.rs2());
self.set_register(inst.rd, if lhs >= rhs { 1 } else { 0 });
self.pc += 1;
}
OpCode::Load => {
Expand Down
2 changes: 1 addition & 1 deletion docs/COMPLETION_STRATEGY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,5 +337,5 @@ Production-ready codebase with complete documentation.
**Status**: Ready to Execute
**Owner**: Development Team
**Start Date**: November 23, 2025
**Target Completion**: End of December 2025
**Target Completion**: Mid-January 2026
**Version**: 1.0.0
4 changes: 2 additions & 2 deletions docs/FINAL_REPORT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# BitCell v0.3 - Final Implementation Report
# BitCell v0.1 - Final Implementation Report

**Date**: November 2025
**Version**: 0.3 (92-95% Complete)
**Version**: 0.1 (92-95% Complete)
**Status**: Production-Ready Foundation

---
Expand Down
6 changes: 3 additions & 3 deletions docs/HOLISTIC_VERIFICATION.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ This document provides a complete verification of the BitCell implementation, co
### 1.1 Cryptographic Primitives ✅

**Module**: `bitcell-crypto`
**Tests**: 39 passing
**Tests**: 27 passing
**Status**: PRODUCTION READY

#### Implementations
Expand Down Expand Up @@ -308,12 +308,12 @@ bitcell-node version

### 2.2 Testing Infrastructure ✅

**Total Tests**: 148 passing
**Total Tests**: 157+ passing
**Test Runtime**: <5 seconds
**Status**: COMPREHENSIVE

#### Test Breakdown
- bitcell-crypto: 39 tests (includes ECVRF, CLSAG)
- bitcell-crypto: 27 tests
- bitcell-ca: 27 tests
- bitcell-ebsl: 27 tests
- bitcell-consensus: 8 tests
Expand Down
2 changes: 1 addition & 1 deletion docs/IMPLEMENTATION_SUMMARY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## 🎉 Major Achievement: 70-80% of TODO Items Completed

From an initial 400+ TODO items representing 18-24 person-months of work, we've successfully implemented the vast majority of critical and important features in a focused development session.
From an initial 400+ TODO items representing 18-24 person-months of work, we've successfully implemented the vast majority of critical and important features during a 3-week development sprint.

---

Expand Down