Refactor: Move difficulty adjustment ownership to lib-consensus (#641)

* Refactor: Move difficulty adjustment ownership to lib-consensus

- Create DifficultyManager in lib-consensus
- Move DifficultyConfig to lib-consensus
- Update BlockchainConsensusCoordinator to use DifficultyManager
- Update Blockchain to delegate difficulty adjustment to Coordinator
- Add governance parameters for difficulty adjustment
- Update documentation and architecture diagrams

* fix: address all Copilot review comments for difficulty refactoring PR

- Remove redundant height > 0 condition in should_adjust()
- Fix double lock acquisition in blockchain.rs adjust_difficulty()
- Add division by zero checks in legacy difficulty calculation
- Make config_mut() private to prevent bypassing validation
- Add min_timespan safeguard against integer division returning 0
- Document complete governance application flow for difficulty params
- Remove duplicate DifficultyManager documentation
- Add comprehensive tests for setter methods
- Add 4 new integration tests for difficulty manager
- Export initialize_consensus_integration_with_difficulty_config()

All tests passing. Fixes address security, correctness, and maintainability issues.

* Fix: Make DifficultyManager handle zero actual_timespan consistently with legacy fallback

- Changed DifficultyManager::adjust_difficulty() to return Ok(None) instead of error when actual_timespan is zero
- This matches the behavior of calculate_difficulty_legacy() which silently returns current difficulty
- Both code paths now handle the edge case identically, preventing consensus divergence
- Added warning log to trace when this rare condition occurs

Fixes inconsistent error handling between coordinator and fallback paths that could cause chain forks when actual_timespan equals zero (identical block timestamps).

---------

Co-authored-by: supertramp <hugo@kode.zone>

Author: PeterGRutherford

Cargo.toml

@@ -34,3 +34,5 @@ codegen-units = 1
 [profile.dev]
 opt-level = 0
 debug = true
+
+[workspace.metadata.dev-tools]

lib-blockchain/docs/api-reference.md

@@ -757,7 +757,7 @@ pub const INITIAL_DIFFICULTY: u64 = 0x00000FFF;
 pub const MAX_BLOCK_SIZE: usize = 1_048_576; // 1MB
 pub const MAX_TRANSACTIONS_PER_BLOCK: usize = 1000;
 pub const TARGET_BLOCK_TIME: u64 = 10; // seconds
-pub const DIFFICULTY_ADJUSTMENT_INTERVAL: u64 = 2016; // blocks
+pub const DIFFICULTY_ADJUSTMENT_INTERVAL: u64 = 2016; // blocks (fallback if consensus coordinator unavailable)
 ```
 
 ### Contract Constants

lib-blockchain/docs/architecture.md

@@ -195,6 +195,7 @@ Coordinates with lib-consensus for:
 
 - **Validator Management**: Register and manage blockchain validators
 - **Block Production**: Coordinate block proposal and validation
+- **Difficulty Management**: Coordinate difficulty adjustment and target calculation
 - **DAO Governance**: On-chain governance with proposal and voting
 - **Reward Distribution**: Distribute consensus rewards to participants
 - **Byzantine Fault Tolerance**: Handle malicious validators and network partitions

lib-blockchain/src/blockchain.rs

@@ -783,34 +783,117 @@ impl Blockchain {
         crate::types::hash::blake3_hash(&data)
     }
 
-    /// Adjust mining difficulty based on block times
+    /// Adjust mining difficulty based on block times.
+    /// 
+    /// This method delegates to the consensus coordinator's DifficultyManager when available,
+    /// falling back to the legacy hardcoded constants for backward compatibility.
+    /// 
+    /// The consensus engine owns the difficulty policy per architectural design.
     fn adjust_difficulty(&mut self) -> Result<()> {
-        if self.height % crate::DIFFICULTY_ADJUSTMENT_INTERVAL != 0 {
+        // Get adjustment parameters from consensus coordinator if available
+        let (adjustment_interval, target_timespan) = if let Some(coordinator) = &self.consensus_coordinator {
+            // Use a single tokio block_in_place to call async methods from sync context
+            // Acquire the coordinator read lock once to avoid race conditions and redundant locking
+            tokio::task::block_in_place(|| {
+                tokio::runtime::Handle::current().block_on(async {
+                    let coord = coordinator.read().await;
+                    let interval = coord.get_difficulty_adjustment_interval().await;
+                    let config = coord.get_difficulty_config().await;
+                    (interval, config.target_timespan)
+                })
+            })
+        } else {
+            // Fallback to hardcoded constants for backward compatibility
+            (crate::DIFFICULTY_ADJUSTMENT_INTERVAL, crate::TARGET_TIMESPAN)
+        };
+
+        // Check if we should adjust at this height
+        if self.height % adjustment_interval != 0 {
             return Ok(());
         }
 
-        if self.height < crate::DIFFICULTY_ADJUSTMENT_INTERVAL {
+        if self.height < adjustment_interval {
             return Ok(());
         }
 
         let current_block = &self.blocks[self.height as usize];
-        let interval_start = &self.blocks[(self.height - crate::DIFFICULTY_ADJUSTMENT_INTERVAL) as usize];
-        
-        let actual_timespan = current_block.timestamp() - interval_start.timestamp();
-        let actual_timespan = actual_timespan.max(crate::TARGET_TIMESPAN / 4).min(crate::TARGET_TIMESPAN * 4);
+        let interval_start = &self.blocks[(self.height - adjustment_interval) as usize];
+        
+        let interval_start_time = interval_start.timestamp();
+        let interval_end_time = current_block.timestamp();
+
+        // Calculate new difficulty using consensus coordinator if available
+        let new_difficulty_bits = if let Some(coordinator) = &self.consensus_coordinator {
+            let result = tokio::task::block_in_place(|| {
+                tokio::runtime::Handle::current().block_on(async {
+                    let coord = coordinator.read().await;
+                    coord.calculate_difficulty_adjustment(
+                        self.height,
+                        self.difficulty.bits(),
+                        interval_start_time,
+                        interval_end_time,
+                    ).await
+                })
+            });
+            
+            match result {
+                Ok(Some(new_bits)) => new_bits,
+                Ok(None) => return Ok(()), // No adjustment needed
+                Err(e) => {
+                    tracing::warn!("Difficulty adjustment via coordinator failed: {}, using fallback", e);
+                    // Fallback to legacy calculation
+                    self.calculate_difficulty_legacy(interval_start_time, interval_end_time, target_timespan)
+                }
+            }
+        } else {
+            // Legacy calculation without coordinator
+            self.calculate_difficulty_legacy(interval_start_time, interval_end_time, target_timespan)
+        };
 
-        let new_difficulty_bits = (self.difficulty.bits() as u64 * crate::TARGET_TIMESPAN / actual_timespan) as u32;
+        let old_difficulty = self.difficulty.bits();
         self.difficulty = Difficulty::from_bits(new_difficulty_bits);
 
         tracing::info!(
             "Difficulty adjusted from {} to {} at height {}",
-            self.difficulty.bits(),
+            old_difficulty,
             new_difficulty_bits,
             self.height
         );
 
         Ok(())
     }
+    
+    /// Legacy difficulty calculation using hardcoded constants.
+    /// Used when consensus coordinator is not available.
+    fn calculate_difficulty_legacy(&self, interval_start_time: u64, interval_end_time: u64, target_timespan: u64) -> u32 {
+        // Defensive check: target_timespan should be validated to be non-zero upstream,
+        // but avoid panicking here if that validation is ever bypassed.
+        if target_timespan == 0 {
+            tracing::warn!(
+                "calculate_difficulty_legacy called with target_timespan = 0; \
+                 returning current difficulty without adjustment"
+            );
+            return self.difficulty.bits();
+        }
+        
+        let actual_timespan = interval_end_time.saturating_sub(interval_start_time);
+        // Clamp to prevent extreme adjustments (4x range)
+        let actual_timespan = actual_timespan
+            .max(target_timespan / 4)
+            .min(target_timespan * 4);
+        
+        // Additional defensive check in case clamping still results in zero
+        // (can happen if target_timespan / 4 == 0 due to integer division with small values)
+        if actual_timespan == 0 {
+            tracing::warn!(
+                "calculate_difficulty_legacy computed actual_timespan = 0 after clamping; \
+                 returning current difficulty without adjustment"
+            );
+            return self.difficulty.bits();
+        }
+        
+        (self.difficulty.bits() as u64 * target_timespan / actual_timespan) as u32
+    }
 
     /// Get the latest block
     pub fn latest_block(&self) -> Option<&Block> {

lib-blockchain/src/integration/consensus_integration.rs

@@ -16,7 +16,8 @@ use lib_consensus::{
     DaoEngine, DaoProposalType, DaoVoteChoice,
     RewardCalculator, RewardRound,
     ConsensusProposal, ConsensusVote, VoteType, ConsensusStep,
-    ConsensusType, ConsensusProof, NoOpBroadcaster
+    ConsensusType, ConsensusProof, NoOpBroadcaster,
+    DifficultyConfig, DifficultyManager,
 };
 use lib_crypto::{Hash, hash_blake3, KeyPair};
 use lib_identity::IdentityId;
@@ -80,6 +81,8 @@ pub struct BlockchainConsensusCoordinator {
     pending_proposals: Arc<RwLock<VecDeque<ConsensusProposal>>>,
     /// Active consensus votes
     active_votes: Arc<RwLock<HashMap<Hash, Vec<ConsensusVote>>>>,
+    /// Difficulty manager (owns difficulty adjustment policy)
+    difficulty_manager: Arc<RwLock<DifficultyManager>>,
 }
 
 // Manual Debug implementation because ConsensusEngine doesn't derive Debug
@@ -107,6 +110,9 @@ impl BlockchainConsensusCoordinator {
         ));
 
         let (event_sender, event_receiver) = mpsc::unbounded_channel();
+        
+        // Initialize difficulty manager with default configuration
+        let difficulty_manager = Arc::new(RwLock::new(DifficultyManager::default()));
 
         Ok(Self {
             consensus_engine,
@@ -119,6 +125,38 @@ impl BlockchainConsensusCoordinator {
             current_round_cache: Arc::new(RwLock::new(None)),
             pending_proposals: Arc::new(RwLock::new(VecDeque::new())),
             active_votes: Arc::new(RwLock::new(HashMap::new())),
+            difficulty_manager,
+        })
+    }
+    
+    /// Create a new blockchain consensus coordinator with custom difficulty configuration
+    pub async fn new_with_difficulty_config(
+        blockchain: Arc<RwLock<Blockchain>>,
+        mempool: Arc<RwLock<Mempool>>,
+        consensus_config: ConsensusConfig,
+        difficulty_config: DifficultyConfig,
+    ) -> Result<Self> {
+        let consensus_engine = Arc::new(RwLock::new(
+            ConsensusEngine::new(consensus_config, Arc::new(NoOpBroadcaster))?
+        ));
+
+        let (event_sender, event_receiver) = mpsc::unbounded_channel();
+        
+        // Initialize difficulty manager with provided configuration
+        let difficulty_manager = Arc::new(RwLock::new(DifficultyManager::new(difficulty_config)));
+
+        Ok(Self {
+            consensus_engine,
+            blockchain,
+            mempool,
+            local_validator_id: None,
+            event_sender,
+            event_receiver: Arc::new(RwLock::new(event_receiver)),
+            is_producing_blocks: false,
+            current_round_cache: Arc::new(RwLock::new(None)),
+            pending_proposals: Arc::new(RwLock::new(VecDeque::new())),
+            active_votes: Arc::new(RwLock::new(HashMap::new())),
+            difficulty_manager,
         })
     }
 
@@ -212,8 +250,68 @@ impl BlockchainConsensusCoordinator {
             current_round_cache: self.current_round_cache.clone(),
             pending_proposals: self.pending_proposals.clone(),
             active_votes: self.active_votes.clone(),
+            difficulty_manager: self.difficulty_manager.clone(),
         }
     }
+    
+    /// Get the difficulty manager
+    pub fn difficulty_manager(&self) -> &Arc<RwLock<DifficultyManager>> {
+        &self.difficulty_manager
+    }
+    
+    /// Get the current difficulty configuration
+    pub async fn get_difficulty_config(&self) -> DifficultyConfig {
+        let manager = self.difficulty_manager.read().await;
+        manager.config().clone()
+    }
+    
+    /// Calculate new difficulty using the consensus-owned algorithm
+    ///
+    /// This is the entry point for blockchain difficulty adjustment.
+    /// The blockchain calls this method and the consensus engine owns the algorithm.
+    pub async fn calculate_difficulty_adjustment(
+        &self,
+        height: u64,
+        current_difficulty: u32,
+        interval_start_time: u64,
+        interval_end_time: u64,
+    ) -> Result<Option<u32>> {
+        let manager = self.difficulty_manager.read().await;
+        manager
+            .adjust_difficulty(height, current_difficulty, interval_start_time, interval_end_time)
+            .map_err(|e| anyhow!("Difficulty adjustment failed: {}", e))
+    }
+    
+    /// Check if difficulty should be adjusted at the given height
+    pub async fn should_adjust_difficulty(&self, height: u64) -> bool {
+        let manager = self.difficulty_manager.read().await;
+        manager.should_adjust(height)
+    }
+    
+    /// Get the initial difficulty value from consensus policy
+    pub async fn get_initial_difficulty(&self) -> u32 {
+        let manager = self.difficulty_manager.read().await;
+        manager.initial_difficulty()
+    }
+    
+    /// Get the difficulty adjustment interval from consensus policy
+    pub async fn get_difficulty_adjustment_interval(&self) -> u64 {
+        let manager = self.difficulty_manager.read().await;
+        manager.adjustment_interval()
+    }
+    
+    /// Apply DAO governance updates to difficulty parameters
+    pub async fn apply_difficulty_governance_update(
+        &self,
+        initial_difficulty: Option<u32>,
+        adjustment_interval: Option<u64>,
+        target_timespan: Option<u64>,
+    ) -> Result<()> {
+        let mut manager = self.difficulty_manager.write().await;
+        manager
+            .apply_governance_update(initial_difficulty, adjustment_interval, target_timespan)
+            .map_err(|e| anyhow!("Failed to apply difficulty governance update: {}", e))
+    }
 
     /// Main consensus event processing loop
     async fn consensus_event_loop(&self) {
@@ -1528,6 +1626,46 @@ pub async fn initialize_consensus_integration(
     Ok(coordinator)
 }
 
+/// Initialize consensus integration with custom difficulty configuration
+/// 
+/// This variant allows specifying a custom `DifficultyConfig` for the blockchain
+/// mining difficulty adjustment policy.
+pub async fn initialize_consensus_integration_with_difficulty_config(
+    blockchain: Arc<RwLock<Blockchain>>,
+    mempool: Arc<RwLock<Mempool>>,
+    consensus_type: ConsensusType,
+    difficulty_config: DifficultyConfig,
+) -> Result<BlockchainConsensusCoordinator> {
+    let consensus_config = ConsensusConfig {
+        consensus_type,
+        min_stake: 1000 * 1_000_000, // 1000 ZHTP minimum stake
+        min_storage: 100 * 1024 * 1024 * 1024, // 100 GB minimum storage
+        max_validators: 100,
+        block_time: 10, // 10 second blocks
+        epoch_length_blocks: 100,
+        propose_timeout: 3000,
+        prevote_timeout: 1000,
+        precommit_timeout: 1000,
+        max_transactions_per_block: 1000,
+        max_difficulty: 0x00000000FFFFFFFF,
+        target_difficulty: 0x00000FFF,
+        byzantine_threshold: 1.0 / 3.0,
+        slash_double_sign: 5,
+        slash_liveness: 1,
+        development_mode: false, // Production mode by default
+    };
+
+    let coordinator = BlockchainConsensusCoordinator::new_with_difficulty_config(
+        blockchain,
+        mempool,
+        consensus_config,
+        difficulty_config,
+    ).await?;
+
+    info!("Consensus integration initialized with custom difficulty config");
+    Ok(coordinator)
+}
+
 /// Create a DAO proposal transaction (delegated to consensus engine)
 pub fn create_dao_proposal_transaction(
     proposer_keypair: &KeyPair,

lib-blockchain/src/lib.rs

@@ -58,10 +58,14 @@ pub use integration::consensus_integration::{
     BlockchainConsensusCoordinator,
     ConsensusStatus,
     initialize_consensus_integration,
+    initialize_consensus_integration_with_difficulty_config,
     create_dao_proposal_transaction,
     create_dao_vote_transaction,
 };
 
+// Re-export difficulty types from lib-consensus for convenience
+pub use lib_consensus::{DifficultyConfig, DifficultyManager, DifficultyError, DifficultyResult};
+
 // Re-export contracts when feature is enabled
 #[cfg(feature = "contracts")]
 pub use contracts::*;