Fix for TypedTx enabling and hash generation. Tweak sig V in RPC. (#272)

* Fix for TypedTx hash generation from RlpView
* Tweaks on sig V field in RPC
* Fix for eip2930 incomming tx

crates/ethcore/src/json_tests/local.rs

@@ -146,7 +146,7 @@ pub fn is_same_block(ref_block: &Block, block: &Unverified) -> bool {
             is_ok = is_ok
                 && match ttype {
                     TypedTxId::Legacy => {
-                        test_exp(tx.original_v() == ref_tx.v.0.as_u64(), "Original Sig V")
+                        test_exp(tx.legacy_v() == ref_tx.v.0.as_u64(), "Original Sig V")
                     }
                     TypedTxId::AccessList => {
                         test_exp(tx.standard_v() as u64 == ref_tx.v.0.as_u64(), "Sig V");

crates/ethcore/src/machine/impls.rs

@@ -469,7 +469,7 @@ impl EthereumMachine {
             .map_err(|e| transaction::Error::InvalidRlp(e.to_string()))?;
 
         match tx.tx_type() {
-            transaction::TypedTxId::AccessList if schedule.eip2930 => {
+            transaction::TypedTxId::AccessList if !schedule.eip2930 => {
                 return Err(transaction::Error::TransactionTypeNotEnabled)
             }
             _ => (),

crates/ethcore/types/src/transaction/transaction.rs

@@ -735,11 +735,19 @@ impl UnverifiedTransaction {
         self.signature.standard_v
     }
 
-    /// The `v` value that appears in the RLP.
-    pub fn original_v(&self) -> u64 {
+    /// The legacy `v` value that contains signatures v and chain_id for replay protection.
+    pub fn legacy_v(&self) -> u64 {
         signature::add_chain_replay_protection(self.signature.standard_v, self.chain_id)
     }
 
+    /// The `v` value that appears in the RLP.
+    pub fn v(&self) -> u64 {
+        match self.unsigned {
+            TypedTransaction::Legacy(_) => self.legacy_v(),
+            _ => self.signature.standard_v as u64,
+        }
+    }
+
     /// The chain ID, or `None` if this is a global transaction.
     pub fn chain_id(&self) -> Option<u64> {
         self.chain_id

crates/ethcore/types/src/views/block.rs

@@ -125,7 +125,8 @@ impl<'a> BlockView<'a> {
     pub fn transaction_hashes(&self) -> Vec<H256> {
         self.transactions_rlp()
             .iter()
-            .map(|rlp| keccak(rlp.as_raw()))
+            .map(TypedTransactionView::new)
+            .map(|t| t.hash())
             .collect()
     }
 

crates/ethcore/types/src/views/body.rs

@@ -106,7 +106,8 @@ impl<'a> BodyView<'a> {
     pub fn transaction_hashes(&self) -> Vec<H256> {
         self.transactions_rlp()
             .iter()
-            .map(|rlp| keccak(rlp.as_raw()))
+            .map(TypedTransactionView::new)
+            .map(|t| t.hash())
             .collect()
     }
 

crates/rpc/src/v1/tests/mocked/signer.rs

@@ -638,7 +638,7 @@ fn should_confirm_sign_transaction_with_rlp() {
         + &format!("\"s\":\"0x{:x}\",", U256::from(signature.s()))
         + &format!("\"standardV\":\"0x{:x}\",", U256::from(t.standard_v()))
         + r#""to":"0xd46e8dd67c5d32be8058bb8eb970870f07244567","transactionIndex":null,"#
-        + &format!("\"v\":\"0x{:x}\",", U256::from(t.original_v()))
+        + &format!("\"v\":\"0x{:x}\",", U256::from(t.v()))
         + r#""value":"0x1""#
         + r#"}},"id":1}"#;
 

crates/rpc/src/v1/tests/mocked/signing.rs

@@ -419,7 +419,7 @@ fn should_add_sign_transaction_to_the_queue() {
         + &format!("\"s\":\"0x{:x}\",", U256::from(signature.s()))
         + &format!("\"standardV\":\"0x{:x}\",", U256::from(t.standard_v()))
         + r#""to":"0xd46e8dd67c5d32be8058bb8eb970870f07244567","transactionIndex":null,"#
-        + &format!("\"v\":\"0x{:x}\",", U256::from(t.original_v()))
+        + &format!("\"v\":\"0x{:x}\",", U256::from(t.v()))
         + r#""value":"0x9184e72a""#
         + r#"}},"id":1}"#;
 

crates/rpc/src/v1/tests/mocked/signing_unsafe.rs

@@ -227,7 +227,7 @@ fn rpc_eth_sign_transaction() {
         + &format!("\"s\":\"0x{:x}\",", U256::from(signature.s()))
         + &format!("\"standardV\":\"0x{:x}\",", U256::from(t.standard_v()))
         + r#""to":"0xd46e8dd67c5d32be8058bb8eb970870f07244567","transactionIndex":null,"#
-        + &format!("\"v\":\"0x{:x}\",", U256::from(t.original_v()))
+        + &format!("\"v\":\"0x{:x}\",", U256::from(t.v()))
         + r#""value":"0x9184e72a""#
         + r#"}},"id":1}"#;
 

crates/rpc/src/v1/types/block.rs

@@ -221,7 +221,7 @@ mod tests {
         let serialized = serde_json::to_string(&t).unwrap();
         assert_eq!(
             serialized,
-            r#"[{"hash":"0x0000000000000000000000000000000000000000000000000000000000000000","nonce":"0x0","blockHash":null,"blockNumber":null,"transactionIndex":null,"from":"0x0000000000000000000000000000000000000000","to":null,"value":"0x0","gasPrice":"0x0","gas":"0x0","input":"0x","creates":null,"raw":"0x","publicKey":null,"chainId":null,"standardV":"0x0","v":"0x0","r":"0x0","s":"0x0","condition":null}]"#
+            r#"[{"hash":"0x0000000000000000000000000000000000000000000000000000000000000000","nonce":"0x0","blockHash":null,"blockNumber":null,"transactionIndex":null,"from":"0x0000000000000000000000000000000000000000","to":null,"value":"0x0","gasPrice":"0x0","gas":"0x0","input":"0x","creates":null,"raw":"0x","publicKey":null,"chainId":null,"v":"0x0","r":"0x0","s":"0x0","condition":null}]"#
         );
 
         let t = BlockTransactions::Hashes(vec![H256::default().into()]);

crates/rpc/src/v1/types/transaction.rs

@@ -22,6 +22,7 @@ use miner;
 use serde::{ser::SerializeStruct, Serialize, Serializer};
 use types::transaction::{
     Action, LocalizedTransaction, PendingTransaction, SignedTransaction, TypedTransaction,
+    TypedTxId,
 };
 use v1::types::{AccessList, Bytes, TransactionCondition};
 
@@ -62,8 +63,9 @@ pub struct Transaction {
     pub public_key: Option<H512>,
     /// The network id of the transaction, if any.
     pub chain_id: Option<U64>,
-    /// The standardised V field of the signature (0 or 1).
-    pub standard_v: U256,
+    /// The standardised V field of the signature (0 or 1). Used by legacy transaction
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub standard_v: Option<U256>,
     /// The standardised V field of the signature.
     pub v: U256,
     /// The R field of the signature.
@@ -191,6 +193,12 @@ impl Transaction {
             None
         };
 
+        let standard_v = if t.tx_type() == TypedTxId::Legacy {
+            Some(t.standard_v())
+        } else {
+            None
+        };
+
         Transaction {
             hash: t.hash(),
             nonce: t.tx().nonce,
@@ -215,8 +223,8 @@ impl Transaction {
             raw: Bytes::new(t.signed.encode()),
             public_key: t.recover_public().ok().map(Into::into),
             chain_id: t.chain_id().map(U64::from),
-            standard_v: t.standard_v().into(),
-            v: t.original_v().into(),
+            standard_v: standard_v.map(Into::into),
+            v: t.v().into(),
             r: signature.r().into(),
             s: signature.s().into(),
             condition: None,
@@ -234,6 +242,12 @@ impl Transaction {
         } else {
             None
         };
+        let standard_v = if t.tx_type() == TypedTxId::Legacy {
+            Some(t.standard_v())
+        } else {
+            None
+        };
+
         Transaction {
             hash: t.hash(),
             nonce: t.tx().nonce,
@@ -258,8 +272,8 @@ impl Transaction {
             raw: t.encode().into(),
             public_key: t.public_key().map(Into::into),
             chain_id: t.chain_id().map(U64::from),
-            standard_v: t.standard_v().into(),
-            v: t.original_v().into(),
+            standard_v: standard_v.map(Into::into),
+            v: t.v().into(),
             r: signature.r().into(),
             s: signature.s().into(),
             condition: None,
@@ -315,7 +329,7 @@ mod tests {
         let serialized = serde_json::to_string(&t).unwrap();
         assert_eq!(
             serialized,
-            r#"{"type":"0x1","hash":"0x0000000000000000000000000000000000000000000000000000000000000000","nonce":"0x0","blockHash":null,"blockNumber":null,"transactionIndex":null,"from":"0x0000000000000000000000000000000000000000","to":null,"value":"0x0","gasPrice":"0x0","gas":"0x0","input":"0x","creates":null,"raw":"0x","publicKey":null,"chainId":null,"standardV":"0x0","v":"0x0","r":"0x0","s":"0x0","condition":null,"accessList":[{"address":"0x0000000000000000000000000000000000000000","storageKeys":[]}]}"#
+            r#"{"type":"0x1","hash":"0x0000000000000000000000000000000000000000000000000000000000000000","nonce":"0x0","blockHash":null,"blockNumber":null,"transactionIndex":null,"from":"0x0000000000000000000000000000000000000000","to":null,"value":"0x0","gasPrice":"0x0","gas":"0x0","input":"0x","creates":null,"raw":"0x","publicKey":null,"chainId":null,"v":"0x0","r":"0x0","s":"0x0","condition":null,"accessList":[{"address":"0x0000000000000000000000000000000000000000","storageKeys":[]}]}"#
         );
     }