Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
9 changes: 0 additions & 9 deletions Roadmap.md

This file was deleted.

2 changes: 1 addition & 1 deletion genesis.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"chain_id": "minichain-default",
"timestamp": 1716880000000,
"difficulty": 4,
"target": "0x0000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF",
"target_block_time": 10000,
"alpha": 0.1,
"initial_supply": 1500000000,
Expand Down
2 changes: 1 addition & 1 deletion main.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def mine_and_process_block(chain, mempool, miner_pk):
receipt_root=calculate_receipt_root(receipts),
receipts=receipts,
miner=miner_pk,
difficulty=chain.current_difficulty,
target=chain.current_target,
)

mined_block = mine_block(block)
Expand Down
20 changes: 9 additions & 11 deletions minichain/block.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def __init__(
previous_hash: str,
transactions: Optional[Sequence[Transaction]] = None,
timestamp: Optional[float] = None,
difficulty: Optional[int] = None,
target: Optional[int] = None,

Copy link
Copy Markdown
Contributor

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 unwrapping Optional, checking for None and handling None in 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 of None.

Often special cases can be handled by using a natural default value instead of None.

state_root: Optional[str] = None,
receipt_root: Optional[str] = None,
receipts: Optional[Sequence[Receipt]] = None,
Expand All @@ -59,7 +59,7 @@ def __init__(
if timestamp is None
else int(timestamp)
)
self.difficulty: Optional[int] = difficulty
self.target: Optional[int] = target
self.nonce: int = 0
self.hash: Optional[str] = None
self.state_root: Optional[str] = state_root
Expand All @@ -83,7 +83,7 @@ def to_header_dict(self):
"state_root": self.state_root,
"receipt_root": self.receipt_root,
"timestamp": self.timestamp,
"difficulty": self.difficulty,
"target": hex(self.target) if self.target is not None else None,
"nonce": self.nonce,
}
# Include miner in header only when present (optional field)
Expand Down Expand Up @@ -130,14 +130,12 @@ def from_dict(cls, payload: dict):
for r_payload in payload.get("receipts", [])
]

# Safely extract and cast difficulty and timestamp if they exist
raw_diff = payload.get("difficulty")
if raw_diff is not None:
parsed_diff = int(raw_diff)
if parsed_diff > 256:
raise ValueError(f"Difficulty too large: {parsed_diff}")
# Safely extract and cast target and timestamp if they exist
raw_target = payload.get("target")
if raw_target is not None:
parsed_target = int(raw_target, 16) if isinstance(raw_target, str) else int(raw_target)
else:
parsed_diff = None
parsed_target = None

raw_ts = payload.get("timestamp")
parsed_ts = int(raw_ts) if raw_ts is not None else None
Expand All @@ -146,7 +144,7 @@ def from_dict(cls, payload: dict):
previous_hash=payload["previous_hash"],
transactions=transactions,
timestamp=parsed_ts,
difficulty=parsed_diff,
target=parsed_target,
state_root=payload.get("state_root"),
receipt_root=payload.get("receipt_root"),
receipts=receipts,
Expand Down
81 changes: 50 additions & 31 deletions minichain/chain.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down Expand Up @@ -88,19 +90,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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Parse protocol targets strictly before validating bounds. int(raw_target) silently converts values such as true and 1.9, while isinstance(block.target, int) also accepts True. Restrict targets to non-boolean integers or valid hex strings, and handle conversion failures consistently.

  • minichain/chain.py#L96-L102: reject booleans/floats instead of coercing them, and catch malformed hex or numeric values before exiting.
  • minichain/chain.py#L30-L34: explicitly reject booleans when validating a received block target.
📍 Affects 1 file
  • minichain/chain.py#L96-L102 (this comment)
  • minichain/chain.py#L30-L34
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@minichain/chain.py` around lines 96 - 102, In minichain/chain.py lines
96-102, update target parsing to accept only non-boolean integers or valid
hexadecimal strings, reject booleans and floats without coercion, and catch
malformed conversion values before logging and exiting. In minichain/chain.py
lines 30-34, update the received block target validation to explicitly exclude
booleans while preserving non-boolean integer validation and existing bounds
checks.

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=[]
Expand Down Expand Up @@ -133,27 +143,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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is a new_avg_block_time being computed? I don't see it.

Potential critical bug.

"""
from .validators import ValidationStatus

Expand All @@ -162,18 +173,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))
Expand All @@ -183,19 +194,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):
"""
Expand All @@ -207,17 +218,21 @@ 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)

return ValidationStatus.VALID

def resolve_conflicts(self, new_chain_list) -> tuple[bool, list]:
Expand Down Expand Up @@ -264,13 +279,16 @@ def resolve_conflicts(self, new_chain_list) -> tuple[bool, list]:
temp_state.chain_id = self.chain_id
temp_state.restore(self._genesis_state_snapshot)

temp_difficulty = proposed_chain[0].difficulty
temp_target = proposed_chain[0].target
temp_avg_block_time = self.target_block_time

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
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, []
Expand All @@ -281,7 +299,8 @@ 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

logger.info("Reorg successful! Switched to new chain tip: Block %s", self.last_block.index)
return True, orphans
3 changes: 3 additions & 0 deletions minichain/network_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,6 @@
MAX_FUTURE_BLOCK_TIME_MS = 15000 # Max allowed ms in the future for a block timestamp
GAS_PER_BYTE = 10 # Cost per byte of state storage written
MAX_CALL_DEPTH = 10 # Maximum depth for cross-contract calls
MAX_TARGET = int("F" * 64, 16)
MIN_TARGET = 1

9 changes: 9 additions & 0 deletions minichain/node_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,12 @@

# Mining Config
MINING_MAX_NONCE = 10_000_000 # Number of hashes to attempt before yielding the mining thread

# Initial nonce range parameters (A and B).
# Miners can configure these to create unique search ranges and avoid overlapping work.
MINING_INITIAL_NONCE_MIN = 0

# Recommended upper limit: 2**32 - 1 (4,294,967,295).
# Keeping the upper limit around 32-bits ensures the nonce string in the JSON block
# doesn't become unnecessarily large, and avoids cross-language serialization issues.
MINING_INITIAL_NONCE_MAX = 2**32 - 1
30 changes: 18 additions & 12 deletions minichain/pow.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import time
import random

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Resolve the random-source contract and Ruff S311 finding.

random.randint is predictable. This does not invalidate a PoW hash, but it does not provide an unpredictable starting point for the overlap-reduction goal. If unpredictability is required, use secrets.randbelow. Otherwise, suppress S311 with a rationale and document that this only reduces overlap.

Proposed fix
-import random
+import secrets
...
-    start_nonce = random.randint(MINING_INITIAL_NONCE_MIN, MINING_INITIAL_NONCE_MAX)
+    start_nonce = MINING_INITIAL_NONCE_MIN + secrets.randbelow(
+        MINING_INITIAL_NONCE_MAX - MINING_INITIAL_NONCE_MIN + 1
+    )

Also applies to: 36-37

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@minichain/pow.py` at line 2, Update the random starting-point generation in
the PoW logic around the random import and lines 36–37 to use secrets.randbelow
when unpredictability is required, replacing random.randint and its import. If
unpredictability is intentionally not required, retain the existing source but
add a narrowly scoped Ruff S311 suppression with a rationale and document that
it only reduces overlap.

Source: Linters/SAST tools

from .serialization import canonical_json_hash
from .node_config import MINING_MAX_NONCE
from .node_config import (
MINING_MAX_NONCE,
MINING_INITIAL_NONCE_MIN,
MINING_INITIAL_NONCE_MAX,
)


class MiningExceededError(Exception):
Expand All @@ -14,7 +19,7 @@ def calculate_hash(block_dict):

def mine_block(
block,
difficulty=None,
target=None,
max_nonce=None,
timeout_seconds=None,
logger=None,
Expand All @@ -23,26 +28,27 @@ 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
start_nonce = random.randint(MINING_INITIAL_NONCE_MIN, MINING_INITIAL_NONCE_MAX)
local_nonce = start_nonce
header_dict = block.to_header_dict() # Construct header dict once outside loop
Comment on lines +31 to 38

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 MAX_TARGET, otherwise mining can succeed for a block add_block must reject.

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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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
from .network_config import MAX_TARGET
target = target if target is not None else block.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["target"] = hex(target) # Construct header dict once outside loop
🧰 Tools
🪛 Ruff (0.16.0)

[warning] 28-28: Avoid specifying long messages outside the exception class

(TRY003)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@minichain/pow.py` around lines 26 - 32, Keep the target override local
throughout the mining operation and assign it to block.target only after mining
succeeds; do not mutate the input before timeout, cancellation, or max-nonce
failures. Extend target validation to reject values above MAX_TARGET while
preserving the positive-integer requirement. Ensure header construction and
hashing use the validated candidate target, and leave block.target unchanged on
every failure path.

start_time = time.monotonic()

if logger:
logger.info(
"Mining block %s (Difficulty: %s)",
"Mining block %s (Target: %s)",
block.index,
difficulty,
target,
)

while True:

# Enforce max_nonce limit before hashing
if local_nonce >= max_nonce:
if local_nonce - start_nonce >= max_nonce:
if logger:
logger.warning("Max nonce exceeded during mining.")
raise MiningExceededError("Mining failed: max_nonce exceeded")
Expand All @@ -56,8 +62,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:
Expand Down
2 changes: 1 addition & 1 deletion tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def test_transaction_fee(self):
index=1,
previous_hash="0",
transactions=[tx],
difficulty=1,
target=int("F"*64, 16),
state_root=self.state.state_root(),
receipt_root=calculate_receipt_root([receipt]),
receipts=[receipt],
Expand Down
4 changes: 2 additions & 2 deletions tests/test_persistence.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def _chain_with_tx(self):
index=1,
previous_hash=bc.last_block.hash,
transactions=[tx],
difficulty=bc.current_difficulty,
target=bc.current_target,
state_root=temp_state.state_root(),
receipt_root=calculate_receipt_root([receipt]),
receipts=[receipt],
Expand Down Expand Up @@ -245,7 +245,7 @@ def test_loaded_chain_can_add_new_block(self):
index=len(restored.chain),
previous_hash=restored.last_block.hash,
transactions=[tx2],
difficulty=restored.current_difficulty,
target=restored.current_target,
state_root=temp_state.state_root(),
receipt_root=calculate_receipt_root([receipt2]),
receipts=[receipt2],
Expand Down
Loading
Loading