feat(lib-blockchain): Add DifficultyConfig struct for adaptive diffic… #652
Conversation
…ulty adjustment - Add DifficultyConfig struct with governance-controlled parameters - Fields: target_timespan, adjustment_interval, min/max_adjustment_factor, last_updated_at_height - Default values: 14 days, 2016 blocks, 4x max adjustment (Bitcoin-style) - Add difficulty_config field to Blockchain struct - Initialize with defaults in Blockchain::new() - Implement serialization/deserialization for persistence - Add get_difficulty_config() getter method - Add comprehensive unit tests for validation and serialization - Ensure backward compatibility with existing genesis blocks - Document in lib-blockchain/docs/architecture.md Resolves #605
There was a problem hiding this comment.
Pull request overview
This PR introduces a DifficultyConfig struct to enable governance-controlled adaptive difficulty adjustment for the blockchain's Proof of Useful Work (PoUW) consensus. The implementation follows Bitcoin-style parameters with configurable timespan, adjustment intervals, and clamping factors that can be modified through DAO proposals.
Key Changes:
- New
DifficultyConfigstruct with validation and helper methods for difficulty adjustment - Integration of
difficulty_configfield into theBlockchainstruct with default initialization - Comprehensive unit tests for configuration validation, serialization, and parameter clamping
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| lib-blockchain/src/types/difficulty.rs | Adds DifficultyConfig struct with fields for governance-controlled difficulty parameters, validation logic, helper methods, and comprehensive unit tests |
| lib-blockchain/src/types/mod.rs | Updates difficulty module exports to include DifficultyConfig and related functions explicitly |
| lib-blockchain/src/blockchain.rs | Adds difficulty_config field to Blockchain struct, initializes with defaults in constructor, and provides getter method |
| lib-blockchain/src/lib.rs | Re-exports DifficultyConfig for external use (redundant with wildcard export) |
| lib-blockchain/docs/architecture.md | Documents the adaptive difficulty adjustment architecture with configuration details and key methods |
| blocks: vec![genesis_block.clone()], | ||
| height: 0, | ||
| difficulty: Difficulty::from_bits(crate::INITIAL_DIFFICULTY), | ||
| difficulty_config: DifficultyConfig::default(), |
There was a problem hiding this comment.
The existing blockchain serialization test (test_blockchain_serialization) does not verify that the difficulty_config field is properly serialized and deserialized. Adding a blockchain field requires updating the test to verify backward compatibility and proper serialization of the new field, especially since this field is critical for governance and consensus operations.
| #[test] | ||
| fn test_difficulty_config_validation() { | ||
| // Valid config should pass | ||
| let valid_config = DifficultyConfig::default(); | ||
| assert!(valid_config.validate().is_ok()); | ||
|
|
||
| // Invalid target_timespan | ||
| let mut invalid = DifficultyConfig::default(); | ||
| invalid.target_timespan = 0; | ||
| assert!(invalid.validate().is_err()); | ||
|
|
||
| // Invalid adjustment_interval | ||
| let mut invalid = DifficultyConfig::default(); | ||
| invalid.adjustment_interval = 0; | ||
| assert!(invalid.validate().is_err()); | ||
|
|
||
| // Invalid min_adjustment_factor | ||
| let mut invalid = DifficultyConfig::default(); | ||
| invalid.min_adjustment_factor = 0; | ||
| assert!(invalid.validate().is_err()); | ||
|
|
||
| // Invalid max_adjustment_factor | ||
| let mut invalid = DifficultyConfig::default(); | ||
| invalid.max_adjustment_factor = 0; | ||
| assert!(invalid.validate().is_err()); | ||
| } |
There was a problem hiding this comment.
The validation test only checks for zero values but doesn't test the upper bound validations defined in the validate method. Specifically, it should test cases where target_timespan exceeds 1 year, adjustment_interval exceeds 1,000,000, or adjustment factors exceed 100 to ensure those validation rules work correctly.
| /// Calculated as target_timespan / adjustment_interval | ||
| pub fn target_block_time(&self) -> u64 { | ||
| if self.adjustment_interval == 0 { | ||
| return 600; // Default to 10 minutes if misconfigured |
There was a problem hiding this comment.
The target_block_time method returns a hardcoded 600 seconds when adjustment_interval is 0, but this creates an inconsistency - the method should either return an error or handle the zero case in a way that's consistent with validation. Since validate() already prevents adjustment_interval from being 0, this fallback may never be reached in validated configs. Consider documenting this defensive check or returning Result to handle invalid states explicitly.
| /// Calculated as target_timespan / adjustment_interval | |
| pub fn target_block_time(&self) -> u64 { | |
| if self.adjustment_interval == 0 { | |
| return 600; // Default to 10 minutes if misconfigured | |
| /// | |
| /// Calculated as `target_timespan / adjustment_interval`. | |
| /// | |
| /// # Panics | |
| /// | |
| /// Panics if `adjustment_interval` is zero. Call [`validate`] on this | |
| /// configuration before using it to ensure parameters are well-formed. | |
| pub fn target_block_time(&self) -> u64 { | |
| if self.adjustment_interval == 0 { | |
| panic!("DifficultyConfig is invalid: adjustment_interval must be greater than 0"); |
…ce setter - Rename min/max_adjustment_factor to max_decrease/increase_factor for clarity - Add tracing warning when adjustment_interval is zero in target_block_time() - Add adjust_difficulty_with_config() that uses DifficultyConfig parameters - Add set_difficulty_config() setter with validation for governance updates - Update documentation and add test for new adjust function Co-Authored-By: Claude Opus 4.5 <[email protected]>
…config-struct-to-blockchain-state
…config-struct-to-blockchain-state
- Add get_difficulty_target_timespan() to retrieve specific field without cloning entire config - Update blockchain.rs to use the new method instead of cloning whole DifficultyConfig - Add documentation to get_difficulty_config() noting that it clones (intended for cases needing full config) - Keep set_difficulty_config() for governance updates as already implemented
- Changed DifficultyConfig::with_params() to return Result<Self, String> - Validates all parameters before creating the configuration - Prevents creation of invalid configs that would fail later - Added test_difficulty_config_with_params_invalid() to test validation - Updated existing test to use .expect() for valid parameters
- Renamed max_decrease_factor to max_difficulty_decrease_factor - Renamed max_increase_factor to max_difficulty_increase_factor - Updated all usages: struct fields, with_params, validate, clamp_timespan - Updated all tests to use new field names - Updated blockchain.rs logging to use new field names These names make it explicit that the fields control difficulty changes, not timespan directly, eliminating confusion in clamp_timespan docs and impl. Tests verify all 7 difficulty config tests still pass.
|
umwelt
deleted the
626-featlib-blockchain-add-difficultyconfig-struct-to-blockchain-state
branch
…ulty adjustment
Resolves #605