Merge pull request ethereum#45 from quilt/review-feedback

Review feedback

Author: lightclient

.gitmodules

@@ -1,4 +1,4 @@
 [submodule "tests/fixtures"]
-	path = tests/fixtures
-	url = https://github.com/ethereum/tests
+    path = tests/fixtures
+    url = https://github.com/ethereum/tests
     branch = develop

README.md

@@ -1,6 +1,6 @@
 # Eth1.0 Specifications
 
-### Description
+## Description
 
 This repository contains various specification related to the Ethereum 1.0 chain, specifically the [pyspec](/src/eth1spec/spec.py), specifications for [network upgrades](/network-upgrades), and the [JSON RPC API](/json-rpc). 
 
@@ -25,6 +25,8 @@ This repository contains various specification related to the Ethereum 1.0 chain
 
 ## Consensus Specification (work-in-progress)
 
+The consensus specification is a python implementation of Ethereum that prioritizes readability and simplicity. It [will] accompanied by both narrative and API level documentation of the various components written in restructured text and rendered using Sphinx....
+
  * [Rendered specification](https://quilt.github.io/eth1.0-specs/)
 
 ## Usage
@@ -87,7 +89,7 @@ This specification aims to be:
 
 1. **Correct** - Describe the _intended_ behavior of the Ethereum blockchain, and any deviation from that is a bug.
 2. **Complete** - Capture the entirety of _consensus critical_ parts of Ethereum.
-3. **Usable** - Prioritize readability, clarity, and plain language over performance and brevity.
+3. **Accessible** - Prioritize readability, clarity, and plain language over performance and brevity.
 
 ### Spelling
 

src/eth1spec/evm/__init__.py

@@ -45,7 +45,7 @@ class Evm:
 
     pc: Uint
     stack: List[U256]
-    memory: bytes
+    memory: bytearray
     code: bytes
     gas_left: U256
     current: Address

src/eth1spec/evm/instructions.py

@@ -41,7 +41,7 @@ def add(evm: Evm) -> None:
     x = pop(evm.stack)
     y = pop(evm.stack)
 
-    val = x + y
+    val = x.wrapping_add(y)
 
     push(evm.stack, val)
 

src/eth1spec/evm/interpreter.py

@@ -65,7 +65,7 @@ def process_call(
     evm = Evm(
         pc=Uint(0),
         stack=[],
-        memory=b"",
+        memory=bytearray(),
         code=env.state[target].code,
         gas_left=gas,
         current=target,

src/eth1spec/spec.py

@@ -171,11 +171,14 @@ def apply_body(
 
     state[coinbase].balance += BLOCK_REWARD
 
+    gas_remaining = block_gas_limit - gas_available
+
     receipts_map = {
         bytes(rlp.encode(Uint(k))): v for (k, v) in enumerate(receipts)
     }
-    receipts_y = trie.map_keys(receipts_map, secured=False)
-    return (block_gas_limit - gas_available), trie.root(receipts_y), state
+    receipt_root = trie.root(trie.map_keys(receipts_map, secured=False))
+
+    return (gas_remaining, receipt_root, state)
 
 
 def process_transaction(
@@ -198,7 +201,7 @@ def process_transaction(
     logs : `List[eth1spec.eth_types.Log]`
         Logs generated during execution.
     """
-    assert verify_transaction(tx)
+    assert validate_transaction(tx)
 
     sender_address = env.origin
     sender = env.state[sender_address]
@@ -210,7 +213,7 @@ def process_transaction(
     assert cost <= sender.balance
     sender.balance -= cost
 
-    gas = tx.gas - intrinsic_cost(tx)
+    gas = tx.gas - calculate_intrinsic_cost(tx)
 
     if tx.to is None:
         raise NotImplementedError()  # TODO
@@ -226,24 +229,24 @@ def process_transaction(
     return (gas_used, logs)
 
 
-def verify_transaction(tx: Transaction) -> bool:
+def validate_transaction(tx: Transaction) -> bool:
     """
     Verifies a transaction.
 
     Parameters
     ----------
     tx : `eth1spec.eth_types.Transaction`
-        Transaction to verify.
+        Transaction to validate.
 
     Returns
     -------
     verified : `bool`
         True if the transaction can be executed, or False otherwise.
     """
-    return intrinsic_cost(tx) <= tx.gas
+    return calculate_intrinsic_cost(tx) <= tx.gas
 
 
-def intrinsic_cost(tx: Transaction) -> Uint:
+def calculate_intrinsic_cost(tx: Transaction) -> Uint:
     """
     Calculates the intrinsic cost of the transaction that is charged before
     execution is instantiated.

src/eth1spec/trie.py

@@ -110,10 +110,13 @@ def map_keys(
 
         nibble_list = bytearray(2 * len(key))
         for i in range(2 * len(key)):
-            if i % 2 == 0:  # even
-                nibble_list[i] = key[i // 2] // 16
+            byte_idx = i // 2
+            if i % 2 == 0:
+                # get upper nibble
+                nibble_list[i] = (key[byte_idx] & 0xF0) >> 4
             else:
-                nibble_list[i] = key[i // 2] % 16
+                # get lower nibble
+                nibble_list[i] = key[byte_idx] & 0x0F
 
         mapped[Bytes(nibble_list)] = value