revamp eip155 transactions signing

evm/utils/transactions.py

@@ -9,67 +9,28 @@
 from evm.exceptions import (
     ValidationError,
 )
-from evm.validation import (
-    validate_gt,
-)
-
 from evm.utils.numeric import (
     is_even,
     int_to_big_endian,
 )
 
 
-def create_transaction_signature(unsigned_txn, private_key):
-    signature = private_key.sign_msg(rlp.encode(unsigned_txn))
-    canonical_v, r, s = signature.vrs
-    v = canonical_v + 27
-    return v, r, s
-
-
-def create_eip155_transaction_signature(unsigned_txn, chain_id, private_key):
-    transaction_parts = rlp.decode(rlp.encode(unsigned_txn))
-    transaction_parts_for_signature = (
-        transaction_parts[:-3] + [int_to_big_endian(chain_id), b'', b'']
-    )
-    message = rlp.encode(transaction_parts_for_signature)
-
-    signature = private_key.sign_msg(message)
-    canonical_v, r, s = signature.vrs
-    v = canonical_v + 27
-    if v == 27:
-        eip155_v = chain_id * 2 + 36
-    else:
-        eip155_v = chain_id * 2 + 35
-    return eip155_v, r, s
-
-
-def validate_transaction_signature(transaction):
-    v, r, s = transaction.v, transaction.r, transaction.s
-    canonical_v = v - 27
-    vrs = (canonical_v, r, s)
-    signature = keys.Signature(vrs=vrs)
-    message = transaction.get_message_for_signing()
-    try:
-        public_key = signature.recover_public_key_from_msg(message)
-    except BadSignature as e:
-        raise ValidationError("Bad Signature: {0}".format(str(e)))
-
-    if not signature.verify_msg(message, public_key):
-        raise ValidationError("Invalid Signature")
+EIP155_CHAIN_ID_OFFSET = 35
+V_OFFSET = 27
 
 
 def is_eip_155_signed_transaction(transaction):
-    if transaction.v >= 35:
+    if transaction.v >= EIP155_CHAIN_ID_OFFSET:
         return True
     else:
         return False
 
 
 def extract_chain_id(v):
     if is_even(v):
-        return (v - 36) // 2
+        return (v - EIP155_CHAIN_ID_OFFSET) // 2
     else:
-        return (v - 35) // 2
+        return (v - EIP155_CHAIN_ID_OFFSET) // 2
 
 
 def extract_signature_v(v):
@@ -79,16 +40,39 @@ def extract_signature_v(v):
         return 27
 
 
-def validate_eip155_transaction_signature(transaction):
-    validate_gt(transaction.v, 34, title="Transaction.v")
+def create_transaction_signature(unsigned_txn, private_key, chain_id=None):
+    transaction_parts = rlp.decode(rlp.encode(unsigned_txn))
+
+    if chain_id:
+        transaction_parts_for_signature = (
+            transaction_parts + [int_to_big_endian(chain_id), b'', b'']
+        )
+    else:
+        transaction_parts_for_signature = transaction_parts
+
+    message = rlp.encode(transaction_parts_for_signature)
+    signature = private_key.sign_msg(message)
+
+    canonical_v, r, s = signature.vrs
+
+    if chain_id:
+        v = canonical_v + chain_id * 2 + EIP155_CHAIN_ID_OFFSET
+    else:
+        v = canonical_v + V_OFFSET
+
+    return v, r, s
 
-    v = extract_signature_v(transaction.v)
+
+def validate_transaction_signature(transaction):
+    if is_eip_155_signed_transaction(transaction):
+        v = extract_signature_v(transaction.v)
+    else:
+        v = transaction.v
 
     canonical_v = v - 27
     vrs = (canonical_v, transaction.r, transaction.s)
     signature = keys.Signature(vrs=vrs)
     message = transaction.get_message_for_signing()
-
     try:
         public_key = signature.recover_public_key_from_msg(message)
     except BadSignature as e:

evm/vm/forks/spurious_dragon/transactions.py

@@ -10,11 +10,8 @@
 )
 from evm.utils.transactions import (
     create_transaction_signature,
-    create_eip155_transaction_signature,
     extract_chain_id,
     is_eip_155_signed_transaction,
-    validate_eip155_transaction_signature,
-    validate_transaction_signature,
 )
 
 
@@ -35,23 +32,18 @@ def get_message_for_signing(self):
                 data=self.data,
             ))
 
-    def check_signature_validity(self):
-        if is_eip_155_signed_transaction(self):
-            validate_eip155_transaction_signature(self)
-        else:
-            validate_transaction_signature(self)
-
     @classmethod
     def create_unsigned_transaction(cls, nonce, gas_price, gas, to, value, data):
         return SpuriousDragonUnsignedTransaction(nonce, gas_price, gas, to, value, data)
 
+    @property
+    def chain_id(self):
+        return extract_chain_id(self)
+
 
 class SpuriousDragonUnsignedTransaction(HomesteadUnsignedTransaction):
     def as_signed_transaction(self, private_key, chain_id=None):
-        if chain_id is None:
-            v, r, s = create_transaction_signature(self, private_key)
-        else:
-            v, r, s = create_eip155_transaction_signature(self, private_key)
+        v, r, s = create_transaction_signature(self, private_key, chain_id=chain_id)
         return SpuriousDragonTransaction(
             nonce=self.nonce,
             gas_price=self.gas_price,

tests/core/transaction-utils/test_transaction_signature_validation.py

@@ -5,6 +5,7 @@
 from eth_utils import (
     decode_hex,
     is_same_address,
+    to_canonical_address,
 )
 
 from eth_keys import keys
@@ -15,54 +16,50 @@
 from evm.vm.forks.homestead.transactions import (
     HomesteadTransaction,
 )
+from evm.vm.forks.spurious_dragon.transactions import (
+    SpuriousDragonTransaction,
+)
 
 from evm.utils.transactions import (
     extract_transaction_sender,
     validate_transaction_signature,
 )
 
 
-from evm.vm.forks.spurious_dragon.transactions import (
-    SpuriousDragonTransaction,
-)
-
-from evm.utils.transactions import (
-    is_eip_155_signed_transaction,
-    validate_eip155_transaction_signature,
-)
+@pytest.fixture(params=['Frontier', 'Homestead', 'SpuriousDragon'])
+def transaction_class(request):
+    if request.param == 'Frontier':
+        return FrontierTransaction
+    elif request.param == 'Homestead':
+        return HomesteadTransaction
+    elif request.param == 'SpuriousDragon':
+        return SpuriousDragonTransaction
+    else:
+        raise AssertionError("Unknown param: {0}".format(request.param))
 
 
-@pytest.mark.parametrize(
-    "txn_class",
-    (FrontierTransaction, HomesteadTransaction),
-)
-def test_pre_EIP155_transaction_signature_validation(txn_class, txn_fixture):
+def test_pre_EIP155_transaction_signature_validation(transaction_class, txn_fixture):
     if txn_fixture['chainId'] is not None:
         pytest.skip("Only testng non-EIP155 transactions")
-    transaction = rlp.decode(decode_hex(txn_fixture['signed']), sedes=txn_class)
+    transaction = rlp.decode(decode_hex(txn_fixture['signed']), sedes=transaction_class)
     validate_transaction_signature(transaction)
     transaction.check_signature_validity()
 
 
 def test_EIP155_transaction_signature_validation(txn_fixture):
     transaction = rlp.decode(decode_hex(txn_fixture['signed']), sedes=SpuriousDragonTransaction)
-    if is_eip_155_signed_transaction(transaction):
-        validate_eip155_transaction_signature(transaction)
-    else:
-        validate_transaction_signature(transaction)
+    validate_transaction_signature(transaction)
     transaction.check_signature_validity()
 
 
-@pytest.mark.parametrize(
-    "txn_class",
-    (FrontierTransaction, HomesteadTransaction),
-)
-def test_pre_EIP155_transaction_sender_extraction(txn_class, txn_fixture):
+def test_pre_EIP155_transaction_sender_extraction(transaction_class, txn_fixture):
     if txn_fixture['chainId'] is not None:
         pytest.skip("Only testng non-EIP155 transactions")
     key = keys.PrivateKey(decode_hex(txn_fixture['key']))
-    transaction = rlp.decode(decode_hex(txn_fixture['signed']), sedes=txn_class)
+    transaction = rlp.decode(decode_hex(txn_fixture['signed']), sedes=transaction_class)
     sender = extract_transaction_sender(transaction)
+
+    assert is_same_address(sender, transaction.sender)
     assert is_same_address(sender, key.public_key.to_canonical_address())
 
 
@@ -72,3 +69,46 @@ def test_EIP155_transaction_sender_extraction(txn_fixture):
     sender = extract_transaction_sender(transaction)
     assert is_same_address(sender, transaction.sender)
     assert is_same_address(sender, key.public_key.to_canonical_address())
+
+
+def test_unsigned_to_signed_transaction(txn_fixture, transaction_class):
+    key = keys.PrivateKey(decode_hex(txn_fixture['key']))
+    unsigned_txn = transaction_class.create_unsigned_transaction(
+        nonce=txn_fixture['nonce'],
+        gas_price=txn_fixture['gasPrice'],
+        gas=txn_fixture['gas'],
+        to=(
+            to_canonical_address(txn_fixture['to'])
+            if txn_fixture['to']
+            else b''
+        ),
+        value=txn_fixture['value'],
+        data=decode_hex(txn_fixture['data']),
+    )
+    signed_txn = unsigned_txn.as_signed_transaction(key)
+
+    assert is_same_address(signed_txn.sender, key.public_key.to_canonical_address())
+
+
+def test_unsigned_to_eip155_signed_transaction(txn_fixture, transaction_class):
+    if txn_fixture['chainId'] is None:
+        pytest.skip('No chain id for EIP155 signing')
+    elif not hasattr(transaction_class, 'chain_id'):
+        pytest.skip('Transaction class is not chain aware')
+
+    key = keys.PrivateKey(decode_hex(txn_fixture['key']))
+    unsigned_txn = transaction_class.create_unsigned_transaction(
+        nonce=txn_fixture['nonce'],
+        gas_price=txn_fixture['gasPrice'],
+        gas=txn_fixture['gas'],
+        to=(
+            to_canonical_address(txn_fixture['to'])
+            if txn_fixture['to']
+            else b''
+        ),
+        value=txn_fixture['value'],
+        data=decode_hex(txn_fixture['data']),
+    )
+    signed_txn = unsigned_txn.as_signed_transaction(key, chain_id=txn_fixture['chainId'])
+
+    assert is_same_address(signed_txn.sender, key.public_key.to_canonical_address())