-
-
Notifications
You must be signed in to change notification settings - Fork 16
Readjusting the difficulty dynamically using PID #52
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.
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
Changes from 10 commits
15542b8
c1f61f9
8ecc46d
48a2943
e7c9879
5d64086
96aa5cb
6e3b6b6
4f1b92b
b206d82
67a5b82
b1377d2
1fb37ae
560a82d
cd7c8d9
0a2d556
55f07b9
a17cefc
6a9e9c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import time | ||
|
|
||
| class PIDDifficultyAdjuster: | ||
| def __init__(self, target_block_time=5, kp=0.5, ki=0.05, kd=0.1): | ||
| self.target_block_time = target_block_time | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the PID controller is a really good addition but right now it relies on floating-point coefficients (0.5, 0.05). In blockchain consensus, floating-point math can lead to chain forks because different CPUs can round floats differently. Could we adapt this to use strictly integer math (like multiplying by 100 and using integer division //) to guarantee 100% determinism across all nodes?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great point — I agree that using floating-point coefficients can introduce non-determinism across nodes, which is risky in a consensus system. We can definitely switch to fixed-point/integer arithmetic (e.g., scaling coefficients by 100 or 1000 and using integer division) to ensure deterministic behavior across all environments. I'll refactor the PID controller to use integer math so that all nodes compute identical results. Thanks for pointing this out!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hey difficulty.py is almost correct but right now it's still using a float multiplier: max_delta = max(1, int(current_difficulty * 0.1)) Since we want exactly 10%, we can skip the float completely and just use integer floor division: max_delta = max(1, current_difficulty // 10) |
||
| # PID Coefficients | ||
| self.kp = kp | ||
| self.ki = ki | ||
| self.kd = kd | ||
|
|
||
| self.integral = 0 | ||
| self.previous_error = 0 | ||
| self.last_block_time = time.monotonic() | ||
|
|
||
| # Limit the integral to prevent "Windup" | ||
| # This stops the difficulty from tanking if the network goes offline | ||
| self.integral_limit = 100 | ||
|
|
||
| # Max percentage the difficulty can change in one block (e.g., 10%) | ||
| self.max_change_factor = 0.1 | ||
|
|
||
| def adjust(self, current_difficulty, actual_block_time=None): | ||
| """ | ||
| Calculates the new difficulty based on the time since the last block. | ||
| """ | ||
| # --- FIX: Handle the case where current_difficulty is None --- | ||
| if current_difficulty is None: | ||
| current_difficulty = 1000 # Default starting difficulty | ||
|
|
||
| if actual_block_time is None: | ||
| now = time.monotonic() | ||
| actual_block_time = now - self.last_block_time | ||
| self.last_block_time = now | ||
|
|
||
| # Error = Goal - Reality | ||
| error = self.target_block_time - actual_block_time | ||
|
|
||
| # Update Integral with clamping (Anti-Windup) | ||
| self.integral = max(min(self.integral + error, self.integral_limit), -self.integral_limit) | ||
|
|
||
| # Derivative: how fast is the error changing? | ||
| derivative = error - self.previous_error | ||
| self.previous_error = error | ||
|
|
||
| # Calculate PID Adjustment | ||
| adjustment = ( | ||
| self.kp * error + | ||
| self.ki * self.integral + | ||
| self.kd * derivative | ||
| ) | ||
|
|
||
| # Apply adjustment with a cap to maintain stability | ||
| # Now current_difficulty is guaranteed to be a number | ||
| max_delta = max(1, int(round(current_difficulty * self.max_change_factor))) | ||
| clamped_adjustment = max(min(adjustment, max_delta), -max_delta) | ||
|
|
||
| delta = int(round(clamped_adjustment)) | ||
| if delta == 0 and clamped_adjustment != 0: | ||
| delta = 1 if clamped_adjustment > 0 else -1 | ||
| new_difficulty = current_difficulty + delta | ||
|
|
||
| # Safety: Difficulty must never drop below 1 | ||
| return max(1, new_difficulty) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In main.py, we only need to pass chain.difficulty into the Block constructor and mine_block() inside the mine_and_process_block function. We don't need the automated run_demo() loop, please keep
main.py interactive and preserve the persistence.py we added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anshulchikhale30-p the testnet demo heading is still there and in many other places as well there are duplicate functions I'll suggest closing this pr and open a new one making only specific changes to main.py as because of merge conflicts a lot of functions have been entered twice and even you have changed the helpbox for no reason not at all required so instead of fixing those will bring more noise to the code I'll suggest opening a new pr with minimal pid specific changes