fix: set signature v value correctly (#2461)

Co-authored-by: Dan Cline <6798349+Rjected@users.noreply.github.com>

crates/primitives/src/transaction/signature.rs

@@ -140,11 +140,9 @@ impl Signature {
 
 #[cfg(test)]
 mod tests {
-    use std::str::FromStr;
-
-    use bytes::BytesMut;
-
     use crate::{Address, Signature, H256, U256};
+    use bytes::BytesMut;
+    use std::str::FromStr;
 
     #[test]
     fn test_payload_len_with_eip155_chain_id() {

crates/primitives/src/transaction/util.rs

@@ -1,17 +1,15 @@
 use crate::{keccak256, Address};
 
 pub(crate) mod secp256k1 {
-    use crate::Signature;
-
     use super::*;
+    use crate::Signature;
+    pub(crate) use ::secp256k1::Error;
     use ::secp256k1::{
         ecdsa::{RecoverableSignature, RecoveryId},
         Message, SecretKey, SECP256K1,
     };
     use revm_primitives::{B256, U256};
 
-    pub(crate) use ::secp256k1::Error;
-
     /// secp256k1 signer recovery
     pub fn recover_signer(sig: &[u8; 65], msg: &[u8; 32]) -> Result<Address, Error> {
         let sig =

crates/rpc/rpc-types/src/eth/transaction/mod.rs

@@ -131,27 +131,32 @@ impl Transaction {
             ),
         };
 
+        let signature = Signature::from_primitive_signature(
+            *signed_tx.signature(),
+            signed_tx.tx_type(),
+            signed_tx.chain_id(),
+        );
+
         Self {
             hash: signed_tx.hash(),
             nonce: U256::from(signed_tx.nonce()),
-            block_hash: None,
-            block_number: None,
-            transaction_index: None,
             from: signer,
             to,
-            value: U256::from(U128::from(*signed_tx.value())),
+            value: U256::from(*signed_tx.value()),
             gas_price,
             max_fee_per_gas,
             max_priority_fee_per_gas: signed_tx.max_priority_fee_per_gas().map(U128::from),
-            signature: Some(Signature::from_primitive_signature(
-                *signed_tx.signature(),
-                signed_tx.chain_id(),
-            )),
+            signature: Some(signature),
             gas: U256::from(signed_tx.gas_limit()),
             input: signed_tx.input().clone(),
             chain_id,
             access_list,
             transaction_type: Some(U64::from(signed_tx.tx_type() as u8)),
+
+            // These fields are set to None because they are not stored as part of the transaction
+            block_hash: None,
+            block_number: None,
+            transaction_index: None,
         }
     }
 }

crates/rpc/rpc-types/src/eth/transaction/signature.rs

@@ -1,5 +1,5 @@
 //! Signature related RPC values
-use reth_primitives::{Signature as PrimitiveSignature, U256};
+use reth_primitives::{Signature as PrimitiveSignature, TxType, U256};
 use serde::{Deserialize, Serialize};
 
 /// Container type for all signature fields in RPC
@@ -9,17 +9,52 @@ pub struct Signature {
     pub r: U256,
     /// The S field of the signature; the point on the curve.
     pub s: U256,
-    // todo: not just 0 or 1, due to eip155
-    /// The standardised recovery id of the signature (0 or 1).
+    /// For EIP-155, EIP-2930 and Blob transactions this is set to the parity (0 for even, 1 for
+    /// odd) of the y-value of the secp256k1 signature.
+    ///
+    /// For legacy transactions, this is the recovery id
+    ///
+    /// See also <https://ethereum.github.io/execution-apis/api-documentation/> and <https://ethereum.org/en/developers/docs/apis/json-rpc/#eth_gettransactionbyhash>
     pub v: U256,
 }
 
 impl Signature {
-    /// Creates a new rpc signature from a [primitive signature](reth_primitives::Signature), using
-    /// the give chain id to compute the signature's recovery id.
+    /// Creates a new rpc signature from a legacy [primitive
+    /// signature](reth_primitives::Signature), using the give chain id to compute the signature's
+    /// recovery id.
     ///
     /// If the chain id is `Some`, the recovery id is computed according to [EIP-155](https://eips.ethereum.org/EIPS/eip-155).
-    pub fn from_primitive_signature(signature: PrimitiveSignature, chain_id: Option<u64>) -> Self {
+    pub(crate) fn from_legacy_primitive_signature(
+        signature: PrimitiveSignature,
+        chain_id: Option<u64>,
+    ) -> Self {
         Self { r: signature.r, s: signature.s, v: U256::from(signature.v(chain_id)) }
     }
+
+    /// Creates a new rpc signature from a non-legacy [primitive
+    /// signature](reth_primitives::Signature). This sets the `v` value to `0` or `1` depending on
+    /// the signature's `odd_y_parity`.
+    pub(crate) fn from_typed_primitive_signature(signature: PrimitiveSignature) -> Self {
+        Self { r: signature.r, s: signature.s, v: U256::from(signature.odd_y_parity as u8) }
+    }
+
+    /// Creates a new rpc signature from a legacy [primitive
+    /// signature](reth_primitives::Signature).
+    ///
+    /// The tx type is used to determine whether or not to use the `chain_id` to compute the
+    /// signature's recovery id.
+    ///
+    /// If the transaction is a legacy transaction, it will use the `chain_id` to compute the
+    /// signature's recovery id. If the transaction is a typed transaction, it will set the `v`
+    /// value to `0` or `1` depending on the signature's `odd_y_parity`.
+    pub fn from_primitive_signature(
+        signature: PrimitiveSignature,
+        tx_type: TxType,
+        chain_id: Option<u64>,
+    ) -> Self {
+        match tx_type {
+            TxType::Legacy => Signature::from_legacy_primitive_signature(signature, chain_id),
+            _ => Signature::from_typed_primitive_signature(signature),
+        }
+    }
 }