addressreview comments

Author: SIDDHANTCOOKIE

minichain/block.py

@@ -1,7 +1,6 @@
 import time
 import hashlib
-from typing import Optional
-from collections.abc import Sequence
+from typing import Sequence, Optional
 
 from .transaction import Transaction
 from .receipt import Receipt
@@ -39,27 +38,27 @@ def __init__(
         self,
         index: int,
         previous_hash: str,
-        transactions: Optional[Sequence[Transaction]] = None,
-        timestamp: Optional[float] = None,
-        target: Optional[int] = None,
+        target: int,
+        transactions: Sequence[Transaction] = (),
+        timestamp: int = 0,
         state_root: Optional[str] = None,
         receipt_root: Optional[str] = None,
-        receipts: Optional[Sequence[Receipt]] = None,
+        receipts: Sequence[Receipt] = (),
         miner: Optional[str] = None,
     ):
         self.index = index
         self.previous_hash = previous_hash
         # Freeze transactions into an immutable tuple to prevent header/body mismatch
-        self.transactions = tuple(transactions) if transactions else ()
-        self.receipts = tuple(receipts) if receipts else ()
+        self.transactions = tuple(transactions)
+        self.receipts = tuple(receipts)
         self.miner = miner
         # Deterministic timestamp (ms)
         self.timestamp: int = (
             round(time.time() * 1000)
-            if timestamp is None
+            if timestamp == 0
             else int(timestamp)
         )
-        self.target: Optional[int] = target
+        self.target: int = target
         self.nonce: int = 0
         self.hash: Optional[str] = None
         self.state_root: Optional[str] = state_root
@@ -83,7 +82,7 @@ def to_header_dict(self):
             "state_root": self.state_root,
             "receipt_root": self.receipt_root,
             "timestamp": self.timestamp,
-            "target": hex(self.target) if self.target is not None else None,
+            "target": hex(self.target),
             "nonce": self.nonce,
         }
         # Include miner in header only when present (optional field)
@@ -138,10 +137,10 @@ def from_dict(cls, payload: dict):
             if not isinstance(parsed_target, int) or parsed_target <= 0 or parsed_target > MAX_TARGET:
                 raise ValueError(f"invalid target in payload: {parsed_target}")
         else:
-            parsed_target = None
+            raise ValueError("missing target in payload")
 
         raw_ts = payload.get("timestamp")
-        parsed_ts = int(raw_ts) if raw_ts is not None else None
+        parsed_ts = int(raw_ts) if raw_ts is not None else 0
         block = cls(
             index=int(payload["index"]),
             previous_hash=payload["previous_hash"],
@@ -153,18 +152,18 @@ def from_dict(cls, payload: dict):
             receipts=receipts,
             miner=payload.get("miner"),
         )
-        block.nonce = int(payload.get("nonce", 0))
+        block.nonce = int(payload.get("nonce") or 0)
         block.hash = payload.get("hash")
 
         # Verify the block hash
         expected_hash = block.compute_hash()
-        if block.hash is not None and block.hash != expected_hash:
+        if block.hash and block.hash != expected_hash:
             raise ValueError("block hash does not match header")
 
         # Recalculate and verify the Merkle root!
         if "merkle_root" in payload and payload["merkle_root"] != block.merkle_root:
             raise ValueError("merkle_root does not match transactions")
-            
+    
         if "receipt_root" in payload:
             expected_receipt_root = calculate_receipt_root(block.receipts)
             if payload["receipt_root"] != expected_receipt_root:
@@ -176,7 +175,7 @@ def from_dict(cls, payload: dict):
     def canonical_payload(self) -> bytes:
         """Returns the full block (header + body) as canonical bytes for networking."""
         # Sanity checks to prevent broadcasting invalid blocks
-        if self.hash is None:
+        if not self.hash:
             raise ValueError("block hash is missing")
         if self.hash != self.compute_hash():
             raise ValueError("block hash does not match header")

minichain/chain.py

@@ -148,6 +148,9 @@ def get_total_work(self, chain_list=None):
         if chain_list is None:
             with self._lock:
                 chain_list = self.chain
+        # The expected number of hashes required to find a block is (1 << 256) / target.
+        # This sums the expected number of hashes for all blocks in the chain,
+        # which represents the total computational work put into the chain.
         return sum((1 << 256) // (block.target or 1) for block in chain_list)
 
     def _next_target(self, target, avg_block_time):

minichain/receipt.py

@@ -1,15 +1,15 @@
-from typing import List, Optional
+from typing import List, Sequence
 
 class Receipt:
     """
     Represents the execution result of a transaction.
     """
-    def __init__(self, tx_hash: str, status: int, gas_used: int = 0, error_message: Optional[str] = None, logs: Optional[List[dict]] = None, contract_address: Optional[str] = None):
+    def __init__(self, tx_hash: str, status: int, gas_used: int = 0, error_message: str = "", logs: Sequence[dict] = (), contract_address: str = ""):
         self.tx_hash = tx_hash
         self.status = status # 1 for success, 0 for failure
         self.gas_used = gas_used
         self.error_message = error_message
-        self.logs = logs or []
+        self.logs = list(logs)
         self.contract_address = contract_address
 
     def to_dict(self) -> dict:
@@ -27,8 +27,8 @@ def from_dict(cls, payload: dict) -> 'Receipt':
         return cls(
             tx_hash=payload["tx_hash"],
             status=payload["status"],
-            gas_used=payload.get("gas_used", 0),
-            error_message=payload.get("error_message"),
-            logs=payload.get("logs", []),
-            contract_address=payload.get("contract_address")
+            gas_used=payload.get("gas_used") or 0,
+            error_message=payload.get("error_message") or "",
+            logs=payload.get("logs") or [],
+            contract_address=payload.get("contract_address") or ""
         )

tests/test_protocol_hardening.py

@@ -16,8 +16,7 @@ def test_canonical_json_is_order_independent(self):
         self.assertEqual(calculate_hash(left), calculate_hash(right))
 
     def test_block_hash_matches_compute_hash(self):
-        block = Block(index=1, previous_hash="abc", transactions=[], timestamp=1234567890)
-        block.target = 2
+        block = Block(index=1, previous_hash="abc", target=2, transactions=[], timestamp=1234567890)
         block.nonce = 7
 
         self.assertEqual(block.compute_hash(), calculate_hash(block.to_header_dict()))

tests/test_target.py

@@ -56,7 +56,7 @@ def test_reorg_target_validation(self):
         chain2.chain[0].hash = chain2.chain[0].compute_hash()
 
         # Chain 2 mines a fast block
-        block1 = Block(1, chain2.last_block.hash, [], timestamp=chain2.last_block.timestamp + 1, target=chain2.current_target, state_root=chain2.state.state_root())
+        block1 = Block(1, chain2.last_block.hash, chain2.current_target, [], timestamp=chain2.last_block.timestamp + 1, state_root=chain2.state.state_root())
         mine_block(block1)
         chain2.add_block(block1)
 
@@ -71,7 +71,7 @@ def test_reorg_target_validation(self):
         # Forging a chain with wrong target should be rejected
         forged_chain = list(chain2.chain)
         # Should be expected_target_fast but we provide start_target instead!
-        forged_block = Block(2, chain2.last_block.hash, [], timestamp=chain2.last_block.timestamp + 1000, target=start_target, state_root=chain2.state.state_root())
+        forged_block = Block(2, chain2.last_block.hash, start_target, [], timestamp=chain2.last_block.timestamp + 1000, state_root=chain2.state.state_root())
         mine_block(forged_block)
         forged_chain.append(forged_block)