-
-
Notifications
You must be signed in to change notification settings - Fork 20
refactor: change PoW difficulty to numeric target hash threshold #133
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 2 commits
99da332
eb58853
0d90055
9611a66
02f8d0b
1c9fa4c
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 |
|---|---|---|
|
|
@@ -27,9 +27,11 @@ def validate_block_link_and_hash(previous_block, block): | |
| if block.hash != expected_hash: | ||
| raise ValueError(f"invalid hash {block.hash}") | ||
|
|
||
| target = "0" * (block.difficulty or 1) | ||
| if not block.hash.startswith(target): | ||
| raise ValueError(f"invalid Proof of Work: hash {block.hash} does not satisfy difficulty {block.difficulty}") | ||
| from .network_config import MAX_TARGET | ||
| if not isinstance(block.target, int) or block.target <= 0 or block.target > MAX_TARGET: | ||
| raise ValueError(f"invalid target: {block.target}") | ||
| if int(block.hash, 16) >= block.target: | ||
| raise ValueError(f"invalid Proof of Work: hash {block.hash} does not satisfy target {block.target}") | ||
|
|
||
| if block.timestamp <= previous_block.timestamp: | ||
| raise ValueError(f"invalid timestamp: {block.timestamp} is not strictly greater than previous block timestamp {previous_block.timestamp}") | ||
|
|
@@ -49,6 +51,9 @@ def __init__(self, genesis_path="genesis.json"): | |
| self.state = State() | ||
| self.chain_id = "minichain-default" | ||
| self._lock = threading.RLock() | ||
| import collections | ||
| self._state_snapshots = collections.OrderedDict() | ||
| self._max_snapshots = 10 | ||
| self._create_genesis_block(genesis_path) | ||
|
|
||
| def _create_genesis_block(self, genesis_path): | ||
|
|
@@ -88,19 +93,27 @@ def _create_genesis_block(self, genesis_path): | |
| self.state.chain_id = self.chain_id | ||
|
|
||
| timestamp = config.get("timestamp") | ||
| difficulty = config.get("difficulty") | ||
| raw_target = config.get("target") | ||
| if raw_target is not None: | ||
| self.current_target = int(raw_target, 16) if isinstance(raw_target, str) else int(raw_target) | ||
| from .network_config import MAX_TARGET | ||
| if not isinstance(self.current_target, int) or self.current_target <= 0 or self.current_target > MAX_TARGET: | ||
| logger.error("Genesis target out of bounds: %s", self.current_target) | ||
| sys.exit(1) | ||
|
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. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Parse protocol targets strictly before validating bounds.
📍 Affects 1 file
🤖 Prompt for AI Agents |
||
| else: | ||
| from .network_config import MAX_TARGET | ||
| self.current_target = MAX_TARGET | ||
|
|
||
| self.target_block_time = config.get("target_block_time", 10000) | ||
| self.alpha = config.get("alpha", 0.1) | ||
| self.current_difficulty = difficulty | ||
| self.avg_block_time = self.target_block_time | ||
|
|
||
| genesis_block = Block( | ||
| index=0, | ||
| previous_hash="0", | ||
| transactions=[], | ||
| timestamp=timestamp, | ||
| difficulty=difficulty, | ||
| target=self.current_target, | ||
| state_root=self.state.state_root(), | ||
| receipt_root=None, | ||
| receipts=[] | ||
|
|
@@ -121,6 +134,7 @@ def _create_genesis_block(self, genesis_path): | |
|
|
||
| # Snapshot the state exactly after genesis allocation for clean reorg rebuilds | ||
| self._genesis_state_snapshot = self.state.snapshot() | ||
| self._state_snapshots[genesis_block.hash] = self.state.snapshot() | ||
|
|
||
| @property | ||
| def last_block(self): | ||
|
|
@@ -133,27 +147,28 @@ def last_block(self): | |
| def get_total_work(self, chain_list=None): | ||
| """ | ||
| Calculates the cumulative PoW of a chain. | ||
| Work is proportional to 2^difficulty. | ||
| Work is inversely proportional to target. | ||
| """ | ||
| if chain_list is None: | ||
| with self._lock: | ||
| chain_list = self.chain | ||
| return sum(2 ** (block.difficulty or 1) for block in chain_list) | ||
| return sum((1 << 256) // (block.target or 1) for block in chain_list) | ||
|
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. Write a comment explaining what this does. |
||
|
|
||
| def _next_difficulty(self, difficulty, avg_block_time): | ||
| """Advance the EMA difficulty control after a block, returning the new value.""" | ||
| def _next_target(self, target, avg_block_time): | ||
| """Advance the EMA target control after a block, returning the new value.""" | ||
| from .network_config import MAX_TARGET, MIN_TARGET | ||
| if avg_block_time > self.target_block_time: | ||
| return max(1, difficulty - 1) | ||
| return min(MAX_TARGET, target + 1) | ||
| if avg_block_time < self.target_block_time: | ||
| return difficulty + 1 | ||
| return difficulty | ||
| return max(MIN_TARGET, target - 1) | ||
| return target | ||
|
|
||
| def _apply_block(self, prev_block, block, state, difficulty, avg_block_time): | ||
| def _apply_block(self, prev_block, block, state, target, avg_block_time): | ||
| """ | ||
| Canonical block-application pipeline shared by add_block and resolve_conflicts. | ||
| Validates `block` against `prev_block` and applies its transactions to `state` | ||
| (mutated in place). On any non-VALID status the caller must discard `state`. | ||
| Returns: (ValidationStatus, new_difficulty, new_avg_block_time) | ||
| Returns: (ValidationStatus, new_target, new_avg_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. Is a Potential critical bug. |
||
| """ | ||
| from .validators import ValidationStatus | ||
|
|
||
|
|
@@ -162,18 +177,18 @@ def _apply_block(self, prev_block, block, state, difficulty, avg_block_time): | |
| except ValueError as exc: | ||
| logger.warning("Block %s rejected: %s", block.index, exc) | ||
| status = ValidationStatus.INVALID if "hash" in str(exc) else ValidationStatus.FAILED | ||
| return status, difficulty, avg_block_time | ||
| return status, target, avg_block_time | ||
|
|
||
| if block.difficulty != difficulty: | ||
| logger.warning("Block %s rejected: Invalid difficulty. Expected %s, got %s", block.index, difficulty, block.difficulty) | ||
| return ValidationStatus.INVALID, difficulty, avg_block_time | ||
| if block.target != target: | ||
| logger.warning("Block %s rejected: Invalid target. Expected %s, got %s", block.index, target, block.target) | ||
| return ValidationStatus.INVALID, target, avg_block_time | ||
|
|
||
| receipts = [] | ||
| for tx in block.transactions: | ||
| status, receipt = state.validate_and_apply_with_status(tx) | ||
| if status != ValidationStatus.VALID: | ||
| logger.warning("Block %s rejected: Transaction failed validation", block.index) | ||
| return status, difficulty, avg_block_time | ||
| return status, target, avg_block_time | ||
| receipts.append(receipt) | ||
|
|
||
| total_fees = sum(getattr(r, 'gas_used', 0) * getattr(tx, 'fee_per_gas', 0) for r, tx in zip(receipts, block.transactions)) | ||
|
|
@@ -183,19 +198,19 @@ def _apply_block(self, prev_block, block, state, difficulty, avg_block_time): | |
| computed_receipt_root = calculate_receipt_root(receipts) | ||
| if block.receipt_root != computed_receipt_root: | ||
| logger.warning("Block %s rejected: Invalid receipt root. Expected %s, got %s", block.index, computed_receipt_root, block.receipt_root) | ||
| return ValidationStatus.INVALID, difficulty, avg_block_time | ||
| return ValidationStatus.INVALID, target, avg_block_time | ||
|
|
||
| if [r.to_dict() for r in block.receipts] != [r.to_dict() for r in receipts]: | ||
| logger.warning("Block %s rejected: Receipts payload mismatch", block.index) | ||
| return ValidationStatus.INVALID, difficulty, avg_block_time | ||
| return ValidationStatus.INVALID, target, avg_block_time | ||
|
|
||
| computed_state_root = state.state_root() | ||
| if block.state_root != computed_state_root: | ||
| logger.warning("Block %s rejected: Invalid state root. Expected %s, got %s", block.index, computed_state_root, block.state_root) | ||
| return ValidationStatus.INVALID, difficulty, avg_block_time | ||
| return ValidationStatus.INVALID, target, avg_block_time | ||
|
|
||
| new_avg = self.alpha * (block.timestamp - prev_block.timestamp) + (1 - self.alpha) * avg_block_time | ||
| return ValidationStatus.VALID, self._next_difficulty(difficulty, new_avg), new_avg | ||
| return ValidationStatus.VALID, self._next_target(target, new_avg), new_avg | ||
|
|
||
| def add_block(self, block): | ||
| """ | ||
|
|
@@ -207,17 +222,25 @@ def add_block(self, block): | |
| with self._lock: | ||
| temp_state = self.state.copy() | ||
| temp_state.chain_id = self.chain_id | ||
| status, new_difficulty, new_avg = self._apply_block( | ||
| self.last_block, block, temp_state, self.current_difficulty, self.avg_block_time | ||
| status, new_target, new_avg = self._apply_block( | ||
| self.last_block, block, temp_state, self.current_target, self.avg_block_time | ||
| ) | ||
| if status != ValidationStatus.VALID: | ||
| return status | ||
|
|
||
| # All transactions valid → commit state and append block | ||
| if hasattr(temp_state.accounts, 'commit'): | ||
| temp_state.accounts.commit() | ||
| temp_state.accounts = temp_state.accounts.backing | ||
| self.state = temp_state | ||
| self.current_difficulty = new_difficulty | ||
| self.current_target = new_target | ||
| self.avg_block_time = new_avg | ||
| self.chain.append(block) | ||
|
|
||
| self._state_snapshots[block.hash] = self.state.snapshot() | ||
| while len(self._state_snapshots) > self._max_snapshots: | ||
| self._state_snapshots.popitem(last=False) | ||
|
|
||
| return ValidationStatus.VALID | ||
|
|
||
| def resolve_conflicts(self, new_chain_list) -> tuple[bool, list]: | ||
|
|
@@ -262,15 +285,34 @@ def resolve_conflicts(self, new_chain_list) -> tuple[bool, list]: | |
|
|
||
| temp_state = State() | ||
| temp_state.chain_id = self.chain_id | ||
| temp_state.restore(self._genesis_state_snapshot) | ||
|
|
||
| temp_difficulty = proposed_chain[0].difficulty | ||
|
|
||
| fork_base_hash = self.chain[fork_idx - 1].hash if fork_idx > 0 else None | ||
|
|
||
| temp_target = proposed_chain[0].target | ||
| temp_avg_block_time = self.target_block_time | ||
|
|
||
| if fork_base_hash and fork_base_hash in self._state_snapshots: | ||
| logger.info("Reorg optimization: Restoring state from in-memory snapshot at block %s", fork_idx - 1) | ||
| temp_state.restore(self._state_snapshots[fork_base_hash]) | ||
|
|
||
| # Fast forward target and avg_block_time without executing state | ||
| for i in range(1, fork_idx): | ||
| block_time = proposed_chain[i].timestamp - proposed_chain[i-1].timestamp | ||
| temp_avg_block_time = self.alpha * block_time + (1 - self.alpha) * temp_avg_block_time | ||
| temp_target = self._next_target(temp_target, temp_avg_block_time) | ||
|
|
||
| start_idx = fork_idx | ||
| else: | ||
| temp_state.restore(self._genesis_state_snapshot) | ||
| start_idx = 1 | ||
|
|
||
| for i in range(1, len(proposed_chain)): | ||
| status, temp_difficulty, temp_avg_block_time = self._apply_block( | ||
| proposed_chain[i - 1], proposed_chain[i], temp_state, temp_difficulty, temp_avg_block_time | ||
| for i in range(start_idx, len(proposed_chain)): | ||
| status, temp_target, temp_avg_block_time = self._apply_block( | ||
| proposed_chain[i - 1], proposed_chain[i], temp_state, temp_target, temp_avg_block_time | ||
| ) | ||
| if hasattr(temp_state.accounts, 'commit'): | ||
| temp_state.accounts.commit() | ||
| temp_state.accounts = temp_state.accounts.backing | ||
| if status != ValidationStatus.VALID: | ||
| logger.warning("Reorg failed at block %s", proposed_chain[i].index) | ||
| return False, [] | ||
|
|
@@ -281,7 +323,13 @@ def resolve_conflicts(self, new_chain_list) -> tuple[bool, list]: | |
|
|
||
| self.chain = proposed_chain | ||
| self.state = temp_state | ||
| self.current_difficulty = temp_difficulty | ||
| self.current_target = temp_target | ||
| self.avg_block_time = temp_avg_block_time | ||
|
|
||
| # Repopulate snapshots for the new chain tip | ||
| self._state_snapshots[self.last_block.hash] = self.state.snapshot() | ||
| while len(self._state_snapshots) > self._max_snapshots: | ||
| self._state_snapshots.popitem(last=False) | ||
|
|
||
| logger.info("Reorg successful! Switched to new chain tip: Block %s", self.last_block.index) | ||
| return True, orphans | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -14,7 +14,7 @@ def calculate_hash(block_dict): | |||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| def mine_block( | ||||||||||||||||||||||||||||||||||||
| block, | ||||||||||||||||||||||||||||||||||||
| difficulty=None, | ||||||||||||||||||||||||||||||||||||
| target=None, | ||||||||||||||||||||||||||||||||||||
| max_nonce=None, | ||||||||||||||||||||||||||||||||||||
| timeout_seconds=None, | ||||||||||||||||||||||||||||||||||||
| logger=None, | ||||||||||||||||||||||||||||||||||||
|
|
@@ -23,20 +23,20 @@ def mine_block( | |||||||||||||||||||||||||||||||||||
| """Mines a block using Proof-of-Work without mutating input block until success.""" | ||||||||||||||||||||||||||||||||||||
| max_nonce = max_nonce if max_nonce is not None else MINING_MAX_NONCE | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| difficulty = difficulty if difficulty is not None else block.difficulty | ||||||||||||||||||||||||||||||||||||
| if not isinstance(difficulty, int) or difficulty <= 0: | ||||||||||||||||||||||||||||||||||||
| raise ValueError("Difficulty must be a positive integer.") | ||||||||||||||||||||||||||||||||||||
| target = target if target is not None else block.target | ||||||||||||||||||||||||||||||||||||
| if not isinstance(target, int) or target <= 0: | ||||||||||||||||||||||||||||||||||||
| raise ValueError("Target must be a positive integer.") | ||||||||||||||||||||||||||||||||||||
| block.target = target | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| target = "0" * difficulty | ||||||||||||||||||||||||||||||||||||
| local_nonce = 0 | ||||||||||||||||||||||||||||||||||||
| header_dict = block.to_header_dict() # Construct header dict once outside loop | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+31
to
38
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. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Keep target overrides atomic and consensus-valid. Line 29 mutates the input before timeout/cancellation/max-nonce failure, contradicting the documented contract; a later retry can mine with the stale override. Also reject targets above Proposed fix def mine_block(...):
+ from .network_config import MAX_TARGET
+
target = target if target is not None else block.target
- if not isinstance(target, int) or target <= 0:
- raise ValueError("Target must be a positive integer.")
- block.target = target
+ if isinstance(target, bool) or not isinstance(target, int) or not 0 < target <= MAX_TARGET:
+ raise ValueError("Target must be an integer within consensus bounds.")
local_nonce = 0
- header_dict = block.to_header_dict()
+ header_dict = block.to_header_dict()
+ header_dict["target"] = hex(target)
...
if int(block_hash, 16) < target:
+ block.target = target
block.nonce = local_nonce📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.16.0)[warning] 28-28: Avoid specifying long messages outside the exception class (TRY003) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| start_time = time.monotonic() | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if logger: | ||||||||||||||||||||||||||||||||||||
| logger.info( | ||||||||||||||||||||||||||||||||||||
| "Mining block %s (Difficulty: %s)", | ||||||||||||||||||||||||||||||||||||
| "Mining block %s (Target: %s)", | ||||||||||||||||||||||||||||||||||||
| block.index, | ||||||||||||||||||||||||||||||||||||
| difficulty, | ||||||||||||||||||||||||||||||||||||
| target, | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| while True: | ||||||||||||||||||||||||||||||||||||
|
|
@@ -56,8 +56,8 @@ def mine_block( | |||||||||||||||||||||||||||||||||||
| header_dict["nonce"] = local_nonce | ||||||||||||||||||||||||||||||||||||
| block_hash = calculate_hash(header_dict) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Check difficulty target | ||||||||||||||||||||||||||||||||||||
| if block_hash.startswith(target): | ||||||||||||||||||||||||||||||||||||
| # Check target | ||||||||||||||||||||||||||||||||||||
| if int(block_hash, 16) < target: | ||||||||||||||||||||||||||||||||||||
| block.nonce = local_nonce # Assign only on success | ||||||||||||||||||||||||||||||||||||
| block.hash = block_hash | ||||||||||||||||||||||||||||||||||||
| if logger: | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
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.
A target is not Optional. Every block must have have a target.
Every dev goes through a phase of abusing the use of
Optional. This leads to anti-patterns in the code, where one needs to be explicitly unwrappingOptional, checking forNoneand handlingNonein ad-hoc ways. I have the impression that this is happening to you currently.Inspect every use of Optional (not only for target and not only for
Block) and ask yourself whether you really need it.For example, for
transactions, one could use an empty sequence of transactions, instead ofNone.Often special cases can be handled by using a natural default value instead of
None.