fix: support hex string in eth_signTypedData_v4 salt (#5033)

.changeset/neat-apes-mix.md

@@ -0,0 +1,5 @@
+---
+"@nomicfoundation/edr": patch
+---
+
+Support hex string as salt in `eth_signTypedData_v4`

crates/edr_provider/src/requests/methods.rs

@@ -219,7 +219,7 @@ pub enum MethodInvocation {
     #[serde(rename = "eth_signTypedData_v4")]
     SignTypedDataV4(
         #[serde(deserialize_with = "crate::requests::serde::deserialize_address")] Address,
-        #[serde(deserialize_with = "crate::requests::serde::deserialize_typed_data")] TypedData,
+        #[serde(deserialize_with = "crate::requests::serde::typed_data::deserialize")] TypedData,
     ),
     /// eth_subscribe
     #[serde(rename = "eth_subscribe")]

crates/edr_provider/src/requests/serde.rs

@@ -5,7 +5,6 @@ use std::{
 };
 
 use edr_eth::{Address, Bytes, U256, U64};
-use ethers_core::types::transaction::eip712::TypedData;
 use serde::{Deserialize, Deserializer, Serialize};
 
 use crate::ProviderError;
@@ -411,17 +410,137 @@ pub(crate) mod storage_value {
     }
 }
 
-/// Helper function for deserializing the payload of an `eth_signTypedData_v4`
-/// request.
-pub(crate) fn deserialize_typed_data<'de, DeserializerT>(
-    deserializer: DeserializerT,
-) -> Result<TypedData, DeserializerT::Error>
-where
-    DeserializerT: Deserializer<'de>,
-{
-    TypedData::deserialize(deserializer).map_err(|_error| {
+/// Helper module for deserializing the payload of an `eth_signTypedData_v4`
+/// request. The types and the deserializer implementation are a patched version
+/// of [`ethers_core`](https://github.com/gakonst/ethers-rs/blob/5394d899adca736a602e316e6f0c06fdb5aa64b9/ethers-core/src/types/transaction/eip712.rs)
+/// in order to support hex strings for the salt parameter.
+/// `ethers_core` is copyright (c) 2020 Georgios Konstantopoulos and is licensed
+/// under the MIT License.
+pub(crate) mod typed_data {
+    use std::collections::BTreeMap;
+
+    use edr_eth::Bytes;
+    use ethers_core::types::transaction::eip712::{EIP712Domain, TypedData, Types};
+    use serde::{Deserialize, Deserializer};
+
+    #[derive(Debug, Deserialize)]
+    #[serde(rename_all = "camelCase")]
+    struct PatchedEIP712Domain {
+        #[serde(default, skip_serializing_if = "Option::is_none")]
+        name: Option<String>,
+
+        #[serde(default, skip_serializing_if = "Option::is_none")]
+        version: Option<String>,
+
+        #[serde(
+            default,
+            skip_serializing_if = "Option::is_none",
+            deserialize_with = "ethers_core::types::serde_helpers::deserialize_stringified_numeric_opt"
+        )]
+        chain_id: Option<ethers_core::types::U256>,
+
+        #[serde(default, skip_serializing_if = "Option::is_none")]
+        verifying_contract: Option<ethers_core::types::Address>,
+
+        // Changed salt from `[u8; 32]` to `Bytes` to support hex strings.
+        #[serde(default, skip_serializing_if = "Option::is_none")]
+        salt: Option<Bytes>,
+    }
+
+    #[derive(Debug, thiserror::Error)]
+    #[error("The salt parameter must be exactly 32 bytes long.")]
+    struct InvalidSaltError;
+
+    impl TryFrom<PatchedEIP712Domain> for EIP712Domain {
+        type Error = InvalidSaltError;
+
+        fn try_from(value: PatchedEIP712Domain) -> Result<Self, Self::Error> {
+            let PatchedEIP712Domain {
+                name,
+                version,
+                chain_id,
+                verifying_contract,
+                salt,
+            } = value;
+
+            let salt: Option<[u8; 32]> = salt
+                .map(|bytes| {
+                    let vec: Vec<u8> = bytes.into();
+                    vec.try_into().map_err(|_error| InvalidSaltError {})
+                })
+                .transpose()?;
+
+            Ok(EIP712Domain {
+                name,
+                version,
+                chain_id,
+                verifying_contract,
+                salt,
+            })
+        }
+    }
+
+    #[derive(Deserialize)]
+    struct TypedDataHelper {
+        domain: PatchedEIP712Domain,
+        types: Types,
+        #[serde(rename = "primaryType")]
+        primary_type: String,
+        message: BTreeMap<String, serde_json::Value>,
+    }
+
+    #[derive(Deserialize)]
+    #[serde(untagged)]
+    enum Type {
+        Val(TypedDataHelper),
+        String(String),
+    }
+
+    fn invalid_json_error<'de, DeserializerT: Deserializer<'de>>(
+        _error: impl std::error::Error,
+    ) -> DeserializerT::Error {
         serde::de::Error::custom("The message parameter is an invalid JSON.".to_string())
-    })
+    }
+
+    /// Helper function for deserializing the payload of an
+    /// `eth_signTypedData_v4` request.
+    pub(crate) fn deserialize<'de, DeserializerT>(
+        deserializer: DeserializerT,
+    ) -> Result<TypedData, DeserializerT::Error>
+    where
+        DeserializerT: Deserializer<'de>,
+    {
+        match Type::deserialize(deserializer).map_err(invalid_json_error::<'de, DeserializerT>)? {
+            Type::Val(v) => {
+                let TypedDataHelper {
+                    domain,
+                    types,
+                    primary_type,
+                    message,
+                } = v;
+                Ok(TypedData {
+                    domain: domain.try_into().map_err(serde::de::Error::custom)?,
+                    types,
+                    primary_type,
+                    message,
+                })
+            }
+            Type::String(s) => {
+                let TypedDataHelper {
+                    domain,
+                    types,
+                    primary_type,
+                    message,
+                } = serde_json::from_str(&s).map_err(invalid_json_error::<'de, DeserializerT>)?;
+                Ok(TypedData {
+                    domain: domain.try_into().map_err(serde::de::Error::custom)?,
+                    types,
+                    primary_type,
+                    message,
+                })
+            }
+        }
+    }
 }
 
 fn invalid_hex<'de, D>(value: &str) -> D::Error

crates/edr_provider/tests/issue_346.rs

@@ -0,0 +1,135 @@
+use edr_provider::{test_utils::create_test_config, NoopLogger, Provider};
+use serde_json::json;
+use tokio::runtime;
+
+// https://github.com/NomicFoundation/edr/issues/346
+#[tokio::test(flavor = "multi_thread")]
+async fn issue_346() -> anyhow::Result<()> {
+    let config = create_test_config();
+    let logger = Box::new(NoopLogger);
+    let subscriber = Box::new(|_event| {});
+    let provider = Provider::new(runtime::Handle::current(), logger, subscriber, config)?;
+
+    // The address has been changed from the repro in the issue to an address that
+    // we have a secret key for in the test config.
+    let request_hex_salt = json!({
+      "method": "eth_signTypedData_v4",
+      "params": [
+        "0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826",
+        {
+          "types": {
+            "UpdateAdminThreshold": [
+              {
+                "name": "threshold",
+                "type": "uint8"
+              },
+              {
+                "name": "nonce",
+                "type": "uint256"
+              }
+            ],
+            "EIP712Domain": [
+              {
+                "name": "name",
+                "type": "string"
+              },
+              {
+                "name": "version",
+                "type": "string"
+              },
+              {
+                "name": "chainId",
+                "type": "uint256"
+              },
+              {
+                "name": "verifyingContract",
+                "type": "address"
+              },
+              {
+                "name": "salt",
+                "type": "bytes32"
+              }
+            ]
+          },
+          "domain": {
+            "name": "Collateral",
+            "version": "2",
+            "chainId": "0x7a69",
+            "verifyingContract": "0xb0279db6a2f1e01fbc8483fccef0be2bc6299cc3",
+            "salt": "0x54c6b2b3ad37d2ee0bf85cf73d4c147b0a1c333627a2cbf9a1bb9ecc1543fc7a"
+          },
+          "primaryType": "UpdateAdminThreshold",
+          "message": {
+            "threshold": "1",
+            "nonce": "0"
+          }
+        }
+      ]
+    });
+
+    assert!(provider
+        .handle_request(serde_json::from_value(request_hex_salt)?)
+        .is_ok());
+
+    #[allow(clippy::zero_prefixed_literal)]
+    let request_array_salt = json!({
+      "method": "eth_signTypedData_v4",
+      "params": [
+        "0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826",
+        {
+          "types": {
+            "UpdateAdminThreshold": [
+              {
+                "name": "threshold",
+                "type": "uint8"
+              },
+              {
+                "name": "nonce",
+                "type": "uint256"
+              }
+            ],
+            "EIP712Domain": [
+              {
+                "name": "name",
+                "type": "string"
+              },
+              {
+                "name": "version",
+                "type": "string"
+              },
+              {
+                "name": "chainId",
+                "type": "uint256"
+              },
+              {
+                "name": "verifyingContract",
+                "type": "address"
+              },
+              {
+                "name": "salt",
+                "type": "bytes32"
+              }
+            ]
+          },
+          "domain": {
+            "name": "Collateral",
+            "version": "2",
+            "chainId": "0x7a69",
+            "verifyingContract": "0xb0279db6a2f1e01fbc8483fccef0be2bc6299cc3",
+            "salt": [84, 198, 178, 179, 73, 5, 10, 38, 1, 48, 2, 47, 1, 6, 0, 23, 0, 8, 1, 4, 9, 62, 03, 49, 61, 87, 58, 04, 1, 7, 52, 22]
+          },
+          "primaryType": "UpdateAdminThreshold",
+          "message": {
+            "threshold": "1",
+            "nonce": "0"
+          }
+        }
+      ]
+    });
+
+    assert!(provider
+        .handle_request(serde_json::from_value(request_array_salt)?)
+        .is_ok());
+
+    Ok(())
+}