From 1998a7a6baa03102c141e09b1f577b502cabf83e Mon Sep 17 00:00:00 2001 From: Agost Biro Date: Wed, 27 Mar 2024 13:05:25 +0100 Subject: [PATCH 1/2] fix: support hex string in `eth_signTypedData_v4` salt --- crates/edr_provider/src/requests/methods.rs | 2 +- crates/edr_provider/src/requests/serde.rs | 141 ++++++++++++++++++-- crates/edr_provider/tests/issue_346.rs | 135 +++++++++++++++++++ 3 files changed, 266 insertions(+), 12 deletions(-) create mode 100644 crates/edr_provider/tests/issue_346.rs diff --git a/crates/edr_provider/src/requests/methods.rs b/crates/edr_provider/src/requests/methods.rs index 3cc0563cd1..049537199f 100644 --- a/crates/edr_provider/src/requests/methods.rs +++ b/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")] diff --git a/crates/edr_provider/src/requests/serde.rs b/crates/edr_provider/src/requests/serde.rs index 0ef02381fe..4bf0a5d36b 100644 --- a/crates/edr_provider/src/requests/serde.rs +++ b/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 -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, + + #[serde(default, skip_serializing_if = "Option::is_none")] + version: Option, + + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "ethers_core::types::serde_helpers::deserialize_stringified_numeric_opt" + )] + chain_id: Option, + + #[serde(default, skip_serializing_if = "Option::is_none")] + verifying_contract: Option, + + // Changed salt from `[u8; 32]` to `Bytes` to support hex strings. + #[serde(default, skip_serializing_if = "Option::is_none")] + salt: Option, + } + + #[derive(Debug, thiserror::Error)] + #[error("The salt parameter must be exactly 32 bytes long.")] + struct InvalidSaltError; + + impl TryFrom for EIP712Domain { + type Error = InvalidSaltError; + + fn try_from(value: PatchedEIP712Domain) -> Result { + let PatchedEIP712Domain { + name, + version, + chain_id, + verifying_contract, + salt, + } = value; + + let salt: Option<[u8; 32]> = salt + .map(|bytes| { + let vec: Vec = 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, + } + + #[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 + 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 diff --git a/crates/edr_provider/tests/issue_346.rs b/crates/edr_provider/tests/issue_346.rs new file mode 100644 index 0000000000..1f721188d4 --- /dev/null +++ b/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(()) +} From c42bd637c16538aa3531bf4cd4fed2ba57274a47 Mon Sep 17 00:00:00 2001 From: Agost Biro Date: Wed, 27 Mar 2024 13:12:16 +0100 Subject: [PATCH 2/2] Add changeset --- .changeset/neat-apes-mix.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/neat-apes-mix.md diff --git a/.changeset/neat-apes-mix.md b/.changeset/neat-apes-mix.md new file mode 100644 index 0000000000..877e456517 --- /dev/null +++ b/.changeset/neat-apes-mix.md @@ -0,0 +1,5 @@ +--- +"@nomicfoundation/edr": patch +--- + +Support hex string as salt in `eth_signTypedData_v4`