From e46b7918a30f0fe1b98dd279957376043bdfbec7 Mon Sep 17 00:00:00 2001 From: Ahmad Kaouk Date: Mon, 3 Jul 2023 00:56:43 +0200 Subject: [PATCH 01/11] Update deserializing data and input fields in TransactionRequest --- .../rpc-core/src/types/transaction_request.rs | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/client/rpc-core/src/types/transaction_request.rs b/client/rpc-core/src/types/transaction_request.rs index a263f4a7f1..46d9c3764a 100644 --- a/client/rpc-core/src/types/transaction_request.rs +++ b/client/rpc-core/src/types/transaction_request.rs @@ -18,11 +18,13 @@ //! `TransactionRequest` type +use std::fmt; + use ethereum::{ AccessListItem, EIP1559TransactionMessage, EIP2930TransactionMessage, LegacyTransactionMessage, }; use ethereum_types::{H160, U256}; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Serialize, Deserializer, de::MapAccess, de::Error}; use crate::types::Bytes; @@ -55,7 +57,7 @@ pub struct TransactionRequest { /// Value of transaction in wei pub value: Option, /// Additional data sent with transaction - #[serde(alias = "input")] + #[serde(deserialize_with = "deserialize_data_input")] pub data: Option, /// Transaction's nonce pub nonce: Option, @@ -119,3 +121,50 @@ impl From for Option { } } } + + +fn deserialize_data_input<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + #[derive(Deserialize)] + #[serde(field_identifier, rename_all = "camelCase")] + enum Field { Data, Input, Other } + + struct DataInputVisitor; + + impl<'de> serde::de::Visitor<'de> for DataInputVisitor { + type Value = Option; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("`data` or `input`") + } + + fn visit_map(self, mut map: V) -> Result + where + V: MapAccess<'de>, + { + let mut value = None; + while let Some(key) = map.next_key()? { + match key { + Field::Data | Field::Input => { + let new_value: Option = map.next_value()?; + match (&value, &new_value) { + (Some(old_value), Some(new_value)) if old_value != new_value => { + return Err(Error::custom("data and input fields are not equal")); + } + _ => (), + } + value = new_value; + } + Field::Other => { + let _: serde::de::IgnoredAny = map.next_value()?; + } + } + } + Ok(value) + } + } + + deserializer.deserialize_struct("TransactionRequest", &["data", "input"], DataInputVisitor) +} From f662046583b9e595db6f66f8247031299f226868 Mon Sep 17 00:00:00 2001 From: Ahmad Kaouk Date: Mon, 3 Jul 2023 11:56:44 +0200 Subject: [PATCH 02/11] Custom deserializing for data and input in CallRequest --- client/rpc-core/src/types/call_request.rs | 3 ++- client/rpc-core/src/types/transaction_request.rs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/client/rpc-core/src/types/call_request.rs b/client/rpc-core/src/types/call_request.rs index 2e014d3475..cc44298610 100644 --- a/client/rpc-core/src/types/call_request.rs +++ b/client/rpc-core/src/types/call_request.rs @@ -23,6 +23,7 @@ use ethereum_types::{H160, H256, U256}; use serde::Deserialize; use crate::types::Bytes; +use super::transaction_request::deserialize_data_input; /// Call request #[derive(Clone, Debug, Default, Eq, PartialEq, Deserialize)] @@ -44,7 +45,7 @@ pub struct CallRequest { /// Value pub value: Option, /// Data - #[serde(alias = "input")] + #[serde(deserialize_with = "deserialize_data_input")] pub data: Option, /// Nonce pub nonce: Option, diff --git a/client/rpc-core/src/types/transaction_request.rs b/client/rpc-core/src/types/transaction_request.rs index 46d9c3764a..178e10dfe0 100644 --- a/client/rpc-core/src/types/transaction_request.rs +++ b/client/rpc-core/src/types/transaction_request.rs @@ -123,7 +123,7 @@ impl From for Option { } -fn deserialize_data_input<'de, D>(deserializer: D) -> Result, D::Error> +pub(crate) fn deserialize_data_input<'de, D>(deserializer: D) -> Result, D::Error> where D: Deserializer<'de>, { @@ -158,7 +158,7 @@ where value = new_value; } Field::Other => { - let _: serde::de::IgnoredAny = map.next_value()?; + let _ = map.next_value()?; } } } From b6dffef6a0a1cf03513028a33b0c71d85803dffe Mon Sep 17 00:00:00 2001 From: Ahmad Kaouk Date: Tue, 4 Jul 2023 11:02:13 +0200 Subject: [PATCH 03/11] Manually implement Deserialize for TransactionRequest and CallRequest --- client/rpc-core/src/types/call_request.rs | 182 +++++++++++++- .../rpc-core/src/types/transaction_request.rs | 226 ++++++++++++++---- 2 files changed, 353 insertions(+), 55 deletions(-) diff --git a/client/rpc-core/src/types/call_request.rs b/client/rpc-core/src/types/call_request.rs index cc44298610..3d7374f083 100644 --- a/client/rpc-core/src/types/call_request.rs +++ b/client/rpc-core/src/types/call_request.rs @@ -20,15 +20,12 @@ use std::collections::BTreeMap; use ethereum::AccessListItem; use ethereum_types::{H160, H256, U256}; -use serde::Deserialize; +use serde::{de::{Visitor, MapAccess, self}, Deserialize, Deserializer}; use crate::types::Bytes; -use super::transaction_request::deserialize_data_input; /// Call request -#[derive(Clone, Debug, Default, Eq, PartialEq, Deserialize)] -#[serde(deny_unknown_fields)] -#[serde(rename_all = "camelCase")] +#[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct CallRequest { /// From pub from: Option, @@ -45,14 +42,14 @@ pub struct CallRequest { /// Value pub value: Option, /// Data - #[serde(deserialize_with = "deserialize_data_input")] pub data: Option, + /// Input + pub input: Option, /// Nonce pub nonce: Option, /// AccessList pub access_list: Option>, /// EIP-2718 type - #[serde(rename = "type")] pub transaction_type: Option, } @@ -74,3 +71,174 @@ pub struct CallStateOverride { /// executing the call. pub state_diff: Option>, } + +impl<'de> Deserialize<'de> for CallRequest { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + #[derive(Deserialize)] + #[serde(field_identifier, rename_all = "camelCase")] + enum Field { + From, + To, + GasPrice, + MaxFeePerGas, + MaxPriorityFeePerGas, + Gas, + Value, + Data, + Input, + Nonce, + AccessList, + Type, + } + + struct CallRequestVisitor; + + impl<'de> Visitor<'de> for CallRequestVisitor { + type Value = CallRequest; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("struct CallRequest") + } + + fn visit_map(self, mut map: V) -> Result + where + V: MapAccess<'de>, + { + let mut from = None; + let mut to = None; + let mut gas_price = None; + let mut max_fee_per_gas = None; + let mut max_priority_fee_per_gas = None; + let mut gas = None; + let mut value = None; + let mut data = None; + let mut input = None; + let mut nonce = None; + let mut access_list = None; + let mut transaction_type = None; + + while let Some(key) = map.next_key()? { + match key { + Field::From => { + if from.is_some() { + return Err(de::Error::duplicate_field("from")); + } + from = Some(map.next_value()?); + } + Field::To => { + if to.is_some() { + return Err(de::Error::duplicate_field("to")); + } + to = Some(map.next_value()?); + } + Field::GasPrice => { + if gas_price.is_some() { + return Err(de::Error::duplicate_field("gasPrice")); + } + gas_price = Some(map.next_value()?); + } + Field::MaxFeePerGas => { + if max_fee_per_gas.is_some() { + return Err(de::Error::duplicate_field("maxFeePerGas")); + } + max_fee_per_gas = Some(map.next_value()?); + } + Field::MaxPriorityFeePerGas => { + if max_priority_fee_per_gas.is_some() { + return Err(de::Error::duplicate_field("maxPriorityFeePerGas")); + } + max_priority_fee_per_gas = Some(map.next_value()?); + } + Field::Gas => { + if gas.is_some() { + return Err(de::Error::duplicate_field("gas")); + } + gas = Some(map.next_value()?); + } + Field::Value => { + if value.is_some() { + return Err(de::Error::duplicate_field("value")); + } + value = Some(map.next_value()?); + } + Field::Data => { + if data.is_some() { + return Err(de::Error::duplicate_field("data")); + } + data = Some(map.next_value()?); + } + Field::Input => { + if input.is_some() { + return Err(de::Error::duplicate_field("input")); + } + input = Some(map.next_value()?); + } + Field::Nonce => { + if nonce.is_some() { + return Err(de::Error::duplicate_field("nonce")); + } + nonce = Some(map.next_value()?); + } + Field::AccessList => { + if access_list.is_some() { + return Err(de::Error::duplicate_field("accessList")); + } + access_list = Some(map.next_value()?); + } + Field::Type => { + if transaction_type.is_some() { + return Err(de::Error::duplicate_field("type")); + } + transaction_type = Some(map.next_value()?); + } + } + } + + match (data.as_ref(), input.as_ref()) { + (Some(data), Some(input)) if data != input => { + return Err(de::Error::custom("data and input must be equal when both are present")) + } + (None, Some(_)) => data = input.take(), + _ => {} + } + + Ok(CallRequest { + from, + to, + gas_price, + max_fee_per_gas, + max_priority_fee_per_gas, + gas, + value, + data, + input, + nonce, + access_list, + transaction_type, + }) + } + } + + deserializer.deserialize_struct( + "CallRequest", + &[ + "from", + "to", + "gasPrice", + "maxFeePerGas", + "maxPriorityFeePerGas", + "gas", + "value", + "data", + "input", + "nonce", + "accessList", + "type", + ], + CallRequestVisitor, + ) + } +} diff --git a/client/rpc-core/src/types/transaction_request.rs b/client/rpc-core/src/types/transaction_request.rs index 178e10dfe0..4cee9604e5 100644 --- a/client/rpc-core/src/types/transaction_request.rs +++ b/client/rpc-core/src/types/transaction_request.rs @@ -17,14 +17,16 @@ // along with this program. If not, see . //! `TransactionRequest` type - use std::fmt; use ethereum::{ AccessListItem, EIP1559TransactionMessage, EIP2930TransactionMessage, LegacyTransactionMessage, }; use ethereum_types::{H160, U256}; -use serde::{Deserialize, Serialize, Deserializer, de::MapAccess, de::Error}; +use serde::{ + de::{self, MapAccess, Visitor}, + Deserialize, Deserializer, Serialize, +}; use crate::types::Bytes; @@ -35,7 +37,7 @@ pub enum TransactionMessage { } /// Transaction request coming from RPC -#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize)] #[serde(deny_unknown_fields)] #[serde(rename_all = "camelCase")] pub struct TransactionRequest { @@ -57,8 +59,9 @@ pub struct TransactionRequest { /// Value of transaction in wei pub value: Option, /// Additional data sent with transaction - #[serde(deserialize_with = "deserialize_data_input")] pub data: Option, + /// Input Data + pub input: Option, /// Transaction's nonce pub nonce: Option, /// Pre-pay to warm storage access. @@ -122,49 +125,176 @@ impl From for Option { } } +impl<'de> Deserialize<'de> for TransactionRequest { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + #[derive(Deserialize)] + #[serde(field_identifier, rename_all = "camelCase")] + enum Field { + From, + To, + GasPrice, + MaxFeePerGas, + MaxPriorityFeePerGas, + Gas, + Value, + Data, + Input, + Nonce, + AccessList, + Type, + } + + struct TransactionRequestVisitor; + + impl<'de> Visitor<'de> for TransactionRequestVisitor { + type Value = TransactionRequest; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("struct TransactionRequest") + } + + fn visit_map(self, mut map: V) -> Result + where + V: MapAccess<'de>, + { + let mut from = None; + let mut to = None; + let mut gas_price = None; + let mut max_fee_per_gas = None; + let mut max_priority_fee_per_gas = None; + let mut gas = None; + let mut value = None; + let mut data = None; + let mut input = None; + let mut nonce = None; + let mut access_list = None; + let mut transaction_type = None; -pub(crate) fn deserialize_data_input<'de, D>(deserializer: D) -> Result, D::Error> -where - D: Deserializer<'de>, -{ - #[derive(Deserialize)] - #[serde(field_identifier, rename_all = "camelCase")] - enum Field { Data, Input, Other } - - struct DataInputVisitor; - - impl<'de> serde::de::Visitor<'de> for DataInputVisitor { - type Value = Option; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("`data` or `input`") - } - - fn visit_map(self, mut map: V) -> Result - where - V: MapAccess<'de>, - { - let mut value = None; - while let Some(key) = map.next_key()? { - match key { - Field::Data | Field::Input => { - let new_value: Option = map.next_value()?; - match (&value, &new_value) { - (Some(old_value), Some(new_value)) if old_value != new_value => { - return Err(Error::custom("data and input fields are not equal")); - } - _ => (), - } - value = new_value; - } - Field::Other => { - let _ = map.next_value()?; - } - } - } - Ok(value) - } - } - - deserializer.deserialize_struct("TransactionRequest", &["data", "input"], DataInputVisitor) + while let Some(key) = map.next_key()? { + match key { + Field::From => { + if from.is_some() { + return Err(de::Error::duplicate_field("from")); + } + from = Some(map.next_value()?); + } + Field::To => { + if to.is_some() { + return Err(de::Error::duplicate_field("to")); + } + to = Some(map.next_value()?); + } + Field::GasPrice => { + if gas_price.is_some() { + return Err(de::Error::duplicate_field("gasPrice")); + } + gas_price = Some(map.next_value()?); + } + Field::MaxFeePerGas => { + if max_fee_per_gas.is_some() { + return Err(de::Error::duplicate_field("maxFeePerGas")); + } + max_fee_per_gas = Some(map.next_value()?); + } + Field::MaxPriorityFeePerGas => { + if max_priority_fee_per_gas.is_some() { + return Err(de::Error::duplicate_field("maxPriorityFeePerGas")); + } + max_priority_fee_per_gas = Some(map.next_value()?); + } + Field::Gas => { + if gas.is_some() { + return Err(de::Error::duplicate_field("gas")); + } + gas = Some(map.next_value()?); + } + Field::Value => { + if value.is_some() { + return Err(de::Error::duplicate_field("value")); + } + value = Some(map.next_value()?); + } + Field::Data => { + if data.is_some() { + return Err(de::Error::duplicate_field("data")); + } + data = Some(map.next_value()?); + } + Field::Input => { + if input.is_some() { + return Err(de::Error::duplicate_field("input")); + } + input = Some(map.next_value()?); + } + Field::Nonce => { + if nonce.is_some() { + return Err(de::Error::duplicate_field("nonce")); + } + nonce = Some(map.next_value()?); + } + Field::AccessList => { + if access_list.is_some() { + return Err(de::Error::duplicate_field("accessList")); + } + access_list = Some(map.next_value()?); + } + Field::Type => { + if transaction_type.is_some() { + return Err(de::Error::duplicate_field("type")); + } + transaction_type = Some(map.next_value()?); + } + } + } + + match (data.as_ref(), input.as_ref()) { + (Some(data), Some(input)) if data != input => { + return Err(de::Error::custom("data and input must be equal when both are present")) + } + (None, Some(_)) => data = input.take(), + _ => {} + } + + Ok(TransactionRequest { + from, + to, + gas_price, + max_fee_per_gas, + max_priority_fee_per_gas, + gas, + value, + data, + input, + nonce, + access_list, + transaction_type, + }) + } + } + + deserializer.deserialize_struct( + "TransactionRequest", + &[ + "from", + "to", + "gasPrice", + "maxFeePerGas", + "maxPriorityFeePerGas", + "gas", + "value", + "data", + "input", + "nonce", + "accessList", + "type", + ], + TransactionRequestVisitor, + ) + } } + +#[cfg(test)] +mod tests {} From 9de6cac084ef9d5447e863e7504b2a1d4adec5b1 Mon Sep 17 00:00:00 2001 From: Ahmad Kaouk Date: Tue, 4 Jul 2023 12:01:12 +0200 Subject: [PATCH 04/11] Add some unit test for the custom deserialization --- client/rpc-core/src/types/call_request.rs | 75 ++++++++++++++++++ .../rpc-core/src/types/transaction_request.rs | 78 ++++++++++++++++++- 2 files changed, 151 insertions(+), 2 deletions(-) diff --git a/client/rpc-core/src/types/call_request.rs b/client/rpc-core/src/types/call_request.rs index 3d7374f083..265eff1ba9 100644 --- a/client/rpc-core/src/types/call_request.rs +++ b/client/rpc-core/src/types/call_request.rs @@ -201,6 +201,8 @@ impl<'de> Deserialize<'de> for CallRequest { (Some(data), Some(input)) if data != input => { return Err(de::Error::custom("data and input must be equal when both are present")) } + // Assume that the data field is the input field if the data field is not present + // and the input field is present. (None, Some(_)) => data = input.take(), _ => {} } @@ -242,3 +244,76 @@ impl<'de> Deserialize<'de> for CallRequest { ) } } + +#[cfg(test)] +mod tests { + use std::str::FromStr; + + use super::*; + use serde_json::json; + + #[test] + fn test_deserialize_call_request() { + let data = json!({ + "from": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", + "to": "0x13fe2d1d3665660d22ff9624b7be0551ee1ac91b", + "gasPrice": "0x10", + "maxFeePerGas": "0x20", + "maxPriorityFeePerGas": "0x30", + "gas": "0x40", + "value": "0x50", + "data": "0x123abc", + "input": "0x123abc", + "nonce": "0x60", + "accessList": [{"address": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", "storageKeys": []}], + "type": "0x70" + }); + + let tr: CallRequest = serde_json::from_value(data).unwrap(); + + assert_eq!( + tr.from.unwrap(), + H160::from_str("0x60be2d1d3665660d22ff9624b7be0551ee1ac91b").unwrap() + ); + assert_eq!( + tr.to.unwrap(), + H160::from_str("0x13fe2d1d3665660d22ff9624b7be0551ee1ac91b").unwrap() + ); + assert_eq!(tr.gas_price.unwrap(), U256::from(0x10)); + assert_eq!(tr.max_fee_per_gas.unwrap(), U256::from(0x20)); + assert_eq!(tr.max_priority_fee_per_gas.unwrap(), U256::from(0x30)); + assert_eq!(tr.gas.unwrap(), U256::from(0x40)); + assert_eq!(tr.value.unwrap(), U256::from(0x50)); + assert_eq!(tr.nonce.unwrap(), U256::from(0x60)); + assert_eq!(tr.access_list.unwrap().len(), 1); + assert_eq!(tr.transaction_type.unwrap(), U256::from(0x70)); + } + + #[test] + fn test_deserialize_call_request_data_input_mismatch_error() { + let data = json!({ + "data": "0xabc2", + "input": "0xdef1", + }); + + let result: Result = serde_json::from_value(data); + + assert!(result.is_err()); + assert_eq!( + result.unwrap_err().to_string(), + "data and input must be equal when both are present" + ); + } + + #[test] + fn test_deserialize_call_request_data_input_equal() { + let data = json!({ + "data": "0xabc2", + "input": "0xabc2", + }); + + let result: Result = serde_json::from_value(data); + + assert!(result.is_ok()); + } +} \ No newline at end of file diff --git a/client/rpc-core/src/types/transaction_request.rs b/client/rpc-core/src/types/transaction_request.rs index 4cee9604e5..2bcb6a768b 100644 --- a/client/rpc-core/src/types/transaction_request.rs +++ b/client/rpc-core/src/types/transaction_request.rs @@ -252,8 +252,12 @@ impl<'de> Deserialize<'de> for TransactionRequest { match (data.as_ref(), input.as_ref()) { (Some(data), Some(input)) if data != input => { - return Err(de::Error::custom("data and input must be equal when both are present")) + return Err(de::Error::custom( + "data and input must be equal when both are present", + )) } + // Assume that the data field is the input field if the data field is not present + // and the input field is present. (None, Some(_)) => data = input.take(), _ => {} } @@ -297,4 +301,74 @@ impl<'de> Deserialize<'de> for TransactionRequest { } #[cfg(test)] -mod tests {} +mod tests { + use std::str::FromStr; + + use super::*; + use serde_json::json; + + #[test] + fn test_deserialize_transaction_request() { + let data = json!({ + "from": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", + "to": "0x13fe2d1d3665660d22ff9624b7be0551ee1ac91b", + "gasPrice": "0x10", + "maxFeePerGas": "0x20", + "maxPriorityFeePerGas": "0x30", + "gas": "0x40", + "value": "0x50", + "data": "0x123abc", + "input": "0x123abc", + "nonce": "0x60", + "accessList": [{"address": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", "storageKeys": []}], + "type": "0x70" + }); + + let tr: TransactionRequest = serde_json::from_value(data).unwrap(); + + assert_eq!( + tr.from.unwrap(), + H160::from_str("0x60be2d1d3665660d22ff9624b7be0551ee1ac91b").unwrap() + ); + assert_eq!( + tr.to.unwrap(), + H160::from_str("0x13fe2d1d3665660d22ff9624b7be0551ee1ac91b").unwrap() + ); + assert_eq!(tr.gas_price.unwrap(), U256::from(0x10)); + assert_eq!(tr.max_fee_per_gas.unwrap(), U256::from(0x20)); + assert_eq!(tr.max_priority_fee_per_gas.unwrap(), U256::from(0x30)); + assert_eq!(tr.gas.unwrap(), U256::from(0x40)); + assert_eq!(tr.value.unwrap(), U256::from(0x50)); + assert_eq!(tr.nonce.unwrap(), U256::from(0x60)); + assert_eq!(tr.access_list.unwrap().len(), 1); + assert_eq!(tr.transaction_type.unwrap(), U256::from(0x70)); + } + + #[test] + fn test_deserialize_transaction_request_data_input_mismatch_error() { + let data = json!({ + "data": "0xabc2", + "input": "0xdef1", + }); + + let result: Result = serde_json::from_value(data); + + assert!(result.is_err()); + assert_eq!( + result.unwrap_err().to_string(), + "data and input must be equal when both are present" + ); + } + + #[test] + fn test_deserialize_transaction_request_data_input_equal() { + let data = json!({ + "data": "0xabc2", + "input": "0xabc2", + }); + + let result: Result = serde_json::from_value(data); + + assert!(result.is_ok()); + } +} From a7d9f61eae58532b1a4bb83668e30e5de55ad103 Mon Sep 17 00:00:00 2001 From: Ahmad Kaouk Date: Tue, 4 Jul 2023 12:02:20 +0200 Subject: [PATCH 05/11] add functional tests --- ts-tests/tests/test-execute.ts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/ts-tests/tests/test-execute.ts b/ts-tests/tests/test-execute.ts index ce0700e214..4a62f9caa4 100644 --- a/ts-tests/tests/test-execute.ts +++ b/ts-tests/tests/test-execute.ts @@ -238,4 +238,29 @@ describeWithFrontier("Frontier RPC (RPC execution)", (context) => { expect(result.result).to.be.equal(TEST_CONTRACT_DEPLOYED_BYTECODE); }); + + step("Deserializes correctly when data and input are equal", async function () { + const result = await customRequest(context.web3, "eth_call", [ + { + from: GENESIS_ACCOUNT, + gas: `0x${(ETH_BLOCK_GAS_LIMIT - 1).toString(16)}`, + data: TEST_CONTRACT_BYTECODE, + input: TEST_CONTRACT_BYTECODE, + }, + ]); + + expect(result.result).to.be.equal(TEST_CONTRACT_DEPLOYED_BYTECODE); + }); + + step("Throws error when data and input are both present and not equal", async function () { + const result = await customRequest(context.web3, "eth_call", [ + { + from: GENESIS_ACCOUNT, + gas: `0x${(ETH_BLOCK_GAS_LIMIT - 1).toString(16)}`, + data: TEST_CONTRACT_BYTECODE, + input: "0x12345678", + }, + ]); + expect((result as any).error.message).to.match(/^data and input must be equal when both are present/); + }); }); From 86ada9825aa1340266c76588ac550a8e1a2727f5 Mon Sep 17 00:00:00 2001 From: Ahmad Kaouk Date: Tue, 4 Jul 2023 12:05:05 +0200 Subject: [PATCH 06/11] fix formatting --- client/rpc-core/src/types/call_request.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/client/rpc-core/src/types/call_request.rs b/client/rpc-core/src/types/call_request.rs index 265eff1ba9..fb9c5b4e0f 100644 --- a/client/rpc-core/src/types/call_request.rs +++ b/client/rpc-core/src/types/call_request.rs @@ -20,7 +20,10 @@ use std::collections::BTreeMap; use ethereum::AccessListItem; use ethereum_types::{H160, H256, U256}; -use serde::{de::{Visitor, MapAccess, self}, Deserialize, Deserializer}; +use serde::{ + de::{self, MapAccess, Visitor}, + Deserialize, Deserializer, +}; use crate::types::Bytes; @@ -199,7 +202,9 @@ impl<'de> Deserialize<'de> for CallRequest { match (data.as_ref(), input.as_ref()) { (Some(data), Some(input)) if data != input => { - return Err(de::Error::custom("data and input must be equal when both are present")) + return Err(de::Error::custom( + "data and input must be equal when both are present", + )) } // Assume that the data field is the input field if the data field is not present // and the input field is present. @@ -316,4 +321,4 @@ mod tests { assert!(result.is_ok()); } -} \ No newline at end of file +} From 2afa2cecc76b329a99e32b5dbeced8a3901b9851 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Tue, 4 Jul 2023 13:42:58 +0200 Subject: [PATCH 07/11] deserializer for aliased fields --- client/rpc-core/src/types/call_request.rs | 254 +++------------------- 1 file changed, 25 insertions(+), 229 deletions(-) diff --git a/client/rpc-core/src/types/call_request.rs b/client/rpc-core/src/types/call_request.rs index fb9c5b4e0f..5404098621 100644 --- a/client/rpc-core/src/types/call_request.rs +++ b/client/rpc-core/src/types/call_request.rs @@ -20,15 +20,32 @@ use std::collections::BTreeMap; use ethereum::AccessListItem; use ethereum_types::{H160, H256, U256}; -use serde::{ - de::{self, MapAccess, Visitor}, - Deserialize, Deserializer, -}; +use serde::{de::Error, Deserialize, Deserializer}; use crate::types::Bytes; +#[derive(Clone, Debug, Default, Eq, PartialEq, Deserialize)] +pub struct CallOrInputData { + data: Option, + input: Option, +} + +fn deserialize_data_or_input<'d, D: Deserializer<'d>>(d: D) -> Result, D::Error> { + let CallOrInputData { data, input } = CallOrInputData::deserialize(d)?; + match (&data, &input) { + (Some(data), Some(input)) => if data == input { + Ok(Some(data.clone())) + } else { + Err(D::Error::custom("Ambiguous value for `data` and `input`".to_string())) + }, + (_, _) => Ok(data.or(input)) + } +} + /// Call request -#[derive(Clone, Debug, Default, Eq, PartialEq)] +#[derive(Clone, Debug, Default, Eq, PartialEq, Deserialize)] +#[serde(deny_unknown_fields)] +#[serde(rename_all = "camelCase")] pub struct CallRequest { /// From pub from: Option, @@ -45,14 +62,14 @@ pub struct CallRequest { /// Value pub value: Option, /// Data + #[serde(deserialize_with = "deserialize_data_or_input", flatten)] pub data: Option, - /// Input - pub input: Option, /// Nonce pub nonce: Option, /// AccessList pub access_list: Option>, /// EIP-2718 type + #[serde(rename = "type")] pub transaction_type: Option, } @@ -75,184 +92,8 @@ pub struct CallStateOverride { pub state_diff: Option>, } -impl<'de> Deserialize<'de> for CallRequest { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - #[derive(Deserialize)] - #[serde(field_identifier, rename_all = "camelCase")] - enum Field { - From, - To, - GasPrice, - MaxFeePerGas, - MaxPriorityFeePerGas, - Gas, - Value, - Data, - Input, - Nonce, - AccessList, - Type, - } - - struct CallRequestVisitor; - - impl<'de> Visitor<'de> for CallRequestVisitor { - type Value = CallRequest; - - fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { - formatter.write_str("struct CallRequest") - } - - fn visit_map(self, mut map: V) -> Result - where - V: MapAccess<'de>, - { - let mut from = None; - let mut to = None; - let mut gas_price = None; - let mut max_fee_per_gas = None; - let mut max_priority_fee_per_gas = None; - let mut gas = None; - let mut value = None; - let mut data = None; - let mut input = None; - let mut nonce = None; - let mut access_list = None; - let mut transaction_type = None; - - while let Some(key) = map.next_key()? { - match key { - Field::From => { - if from.is_some() { - return Err(de::Error::duplicate_field("from")); - } - from = Some(map.next_value()?); - } - Field::To => { - if to.is_some() { - return Err(de::Error::duplicate_field("to")); - } - to = Some(map.next_value()?); - } - Field::GasPrice => { - if gas_price.is_some() { - return Err(de::Error::duplicate_field("gasPrice")); - } - gas_price = Some(map.next_value()?); - } - Field::MaxFeePerGas => { - if max_fee_per_gas.is_some() { - return Err(de::Error::duplicate_field("maxFeePerGas")); - } - max_fee_per_gas = Some(map.next_value()?); - } - Field::MaxPriorityFeePerGas => { - if max_priority_fee_per_gas.is_some() { - return Err(de::Error::duplicate_field("maxPriorityFeePerGas")); - } - max_priority_fee_per_gas = Some(map.next_value()?); - } - Field::Gas => { - if gas.is_some() { - return Err(de::Error::duplicate_field("gas")); - } - gas = Some(map.next_value()?); - } - Field::Value => { - if value.is_some() { - return Err(de::Error::duplicate_field("value")); - } - value = Some(map.next_value()?); - } - Field::Data => { - if data.is_some() { - return Err(de::Error::duplicate_field("data")); - } - data = Some(map.next_value()?); - } - Field::Input => { - if input.is_some() { - return Err(de::Error::duplicate_field("input")); - } - input = Some(map.next_value()?); - } - Field::Nonce => { - if nonce.is_some() { - return Err(de::Error::duplicate_field("nonce")); - } - nonce = Some(map.next_value()?); - } - Field::AccessList => { - if access_list.is_some() { - return Err(de::Error::duplicate_field("accessList")); - } - access_list = Some(map.next_value()?); - } - Field::Type => { - if transaction_type.is_some() { - return Err(de::Error::duplicate_field("type")); - } - transaction_type = Some(map.next_value()?); - } - } - } - - match (data.as_ref(), input.as_ref()) { - (Some(data), Some(input)) if data != input => { - return Err(de::Error::custom( - "data and input must be equal when both are present", - )) - } - // Assume that the data field is the input field if the data field is not present - // and the input field is present. - (None, Some(_)) => data = input.take(), - _ => {} - } - - Ok(CallRequest { - from, - to, - gas_price, - max_fee_per_gas, - max_priority_fee_per_gas, - gas, - value, - data, - input, - nonce, - access_list, - transaction_type, - }) - } - } - - deserializer.deserialize_struct( - "CallRequest", - &[ - "from", - "to", - "gasPrice", - "maxFeePerGas", - "maxPriorityFeePerGas", - "gas", - "value", - "data", - "input", - "nonce", - "accessList", - "type", - ], - CallRequestVisitor, - ) - } -} - #[cfg(test)] mod tests { - use std::str::FromStr; use super::*; use serde_json::json; @@ -274,51 +115,6 @@ mod tests { "type": "0x70" }); - let tr: CallRequest = serde_json::from_value(data).unwrap(); - - assert_eq!( - tr.from.unwrap(), - H160::from_str("0x60be2d1d3665660d22ff9624b7be0551ee1ac91b").unwrap() - ); - assert_eq!( - tr.to.unwrap(), - H160::from_str("0x13fe2d1d3665660d22ff9624b7be0551ee1ac91b").unwrap() - ); - assert_eq!(tr.gas_price.unwrap(), U256::from(0x10)); - assert_eq!(tr.max_fee_per_gas.unwrap(), U256::from(0x20)); - assert_eq!(tr.max_priority_fee_per_gas.unwrap(), U256::from(0x30)); - assert_eq!(tr.gas.unwrap(), U256::from(0x40)); - assert_eq!(tr.value.unwrap(), U256::from(0x50)); - assert_eq!(tr.nonce.unwrap(), U256::from(0x60)); - assert_eq!(tr.access_list.unwrap().len(), 1); - assert_eq!(tr.transaction_type.unwrap(), U256::from(0x70)); - } - - #[test] - fn test_deserialize_call_request_data_input_mismatch_error() { - let data = json!({ - "data": "0xabc2", - "input": "0xdef1", - }); - - let result: Result = serde_json::from_value(data); - - assert!(result.is_err()); - assert_eq!( - result.unwrap_err().to_string(), - "data and input must be equal when both are present" - ); - } - - #[test] - fn test_deserialize_call_request_data_input_equal() { - let data = json!({ - "data": "0xabc2", - "input": "0xabc2", - }); - - let result: Result = serde_json::from_value(data); - - assert!(result.is_ok()); + let _: CallRequest = serde_json::from_value(data).unwrap(); } } From 04f748ce95688bb47b5fb8ae54d0a37c567e39db Mon Sep 17 00:00:00 2001 From: Ahmad Kaouk Date: Tue, 4 Jul 2023 14:12:41 +0200 Subject: [PATCH 08/11] custom deserializer for data and input fields in TransactionRequest --- client/rpc-core/src/types/call_request.rs | 79 ++++- .../rpc-core/src/types/transaction_request.rs | 281 ++++-------------- 2 files changed, 139 insertions(+), 221 deletions(-) diff --git a/client/rpc-core/src/types/call_request.rs b/client/rpc-core/src/types/call_request.rs index 5404098621..53766d4d8e 100644 --- a/client/rpc-core/src/types/call_request.rs +++ b/client/rpc-core/src/types/call_request.rs @@ -25,12 +25,12 @@ use serde::{de::Error, Deserialize, Deserializer}; use crate::types::Bytes; #[derive(Clone, Debug, Default, Eq, PartialEq, Deserialize)] -pub struct CallOrInputData { +pub(crate) struct CallOrInputData { data: Option, input: Option, } -fn deserialize_data_or_input<'d, D: Deserializer<'d>>(d: D) -> Result, D::Error> { +pub(crate) fn deserialize_data_or_input<'d, D: Deserializer<'d>>(d: D) -> Result, D::Error> { let CallOrInputData { data, input } = CallOrInputData::deserialize(d)?; match (&data, &input) { (Some(data), Some(input)) => if data == input { @@ -99,7 +99,74 @@ mod tests { use serde_json::json; #[test] - fn test_deserialize_call_request() { + fn test_deserialize_with_only_input() { + let data = json!({ + "from": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", + "to": "0x13fe2d1d3665660d22ff9624b7be0551ee1ac91b", + "gasPrice": "0x10", + "maxFeePerGas": "0x20", + "maxPriorityFeePerGas": "0x30", + "gas": "0x40", + "value": "0x50", + "input": "0x123abc", + "nonce": "0x60", + "accessList": [{"address": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", "storageKeys": []}], + "type": "0x70" + }); + + let request: Result = serde_json::from_value(data); + assert!(request.is_ok()); + + let request = request.unwrap(); + assert_eq!(request.data, Some(Bytes::from(vec![0x12, 0x3a, 0xbc]))); + } + + #[test] + fn test_deserialize_with_only_data() { + let data = json!({ + "from": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", + "to": "0x13fe2d1d3665660d22ff9624b7be0551ee1ac91b", + "gasPrice": "0x10", + "maxFeePerGas": "0x20", + "maxPriorityFeePerGas": "0x30", + "gas": "0x40", + "value": "0x50", + "data": "0x123abc", + "nonce": "0x60", + "accessList": [{"address": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", "storageKeys": []}], + "type": "0x70" + }); + + let request: Result = serde_json::from_value(data); + assert!(request.is_ok()); + + let request = request.unwrap(); + assert_eq!(request.data, Some(Bytes::from(vec![0x12, 0x3a, 0xbc]))); + } + + #[test] + fn test_deserialize_with_data_and_input_mismatch() { + let data = json!({ + "from": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", + "to": "0x13fe2d1d3665660d22ff9624b7be0551ee1ac91b", + "gasPrice": "0x10", + "maxFeePerGas": "0x20", + "maxPriorityFeePerGas": "0x30", + "gas": "0x40", + "value": "0x50", + "data": "0x123abc", + "input": "0x456def", + "nonce": "0x60", + "accessList": [{"address": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", "storageKeys": []}], + "type": "0x70" + }); + + let request: Result = serde_json::from_value(data); + assert!(request.is_err()); + } + + #[test] + fn test_deserialize_with_data_and_input_equal() { let data = json!({ "from": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", "to": "0x13fe2d1d3665660d22ff9624b7be0551ee1ac91b", @@ -115,6 +182,10 @@ mod tests { "type": "0x70" }); - let _: CallRequest = serde_json::from_value(data).unwrap(); + let request: Result = serde_json::from_value(data); + assert!(request.is_ok()); + + let request = request.unwrap(); + assert_eq!(request.data, Some(Bytes::from(vec![0x12, 0x3a, 0xbc]))); } } diff --git a/client/rpc-core/src/types/transaction_request.rs b/client/rpc-core/src/types/transaction_request.rs index 2bcb6a768b..1d117fb2b4 100644 --- a/client/rpc-core/src/types/transaction_request.rs +++ b/client/rpc-core/src/types/transaction_request.rs @@ -17,19 +17,16 @@ // along with this program. If not, see . //! `TransactionRequest` type -use std::fmt; - use ethereum::{ AccessListItem, EIP1559TransactionMessage, EIP2930TransactionMessage, LegacyTransactionMessage, }; use ethereum_types::{H160, U256}; -use serde::{ - de::{self, MapAccess, Visitor}, - Deserialize, Deserializer, Serialize, -}; +use serde::{Deserialize, Serialize}; use crate::types::Bytes; +use super::call_request::deserialize_data_or_input; + pub enum TransactionMessage { Legacy(LegacyTransactionMessage), EIP2930(EIP2930TransactionMessage), @@ -37,7 +34,7 @@ pub enum TransactionMessage { } /// Transaction request coming from RPC -#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize)] +#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] #[serde(rename_all = "camelCase")] pub struct TransactionRequest { @@ -59,9 +56,8 @@ pub struct TransactionRequest { /// Value of transaction in wei pub value: Option, /// Additional data sent with transaction + #[serde(deserialize_with = "deserialize_data_or_input", flatten)] pub data: Option, - /// Input Data - pub input: Option, /// Transaction's nonce pub nonce: Option, /// Pre-pay to warm storage access. @@ -125,190 +121,15 @@ impl From for Option { } } -impl<'de> Deserialize<'de> for TransactionRequest { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - #[derive(Deserialize)] - #[serde(field_identifier, rename_all = "camelCase")] - enum Field { - From, - To, - GasPrice, - MaxFeePerGas, - MaxPriorityFeePerGas, - Gas, - Value, - Data, - Input, - Nonce, - AccessList, - Type, - } - - struct TransactionRequestVisitor; - - impl<'de> Visitor<'de> for TransactionRequestVisitor { - type Value = TransactionRequest; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("struct TransactionRequest") - } - - fn visit_map(self, mut map: V) -> Result - where - V: MapAccess<'de>, - { - let mut from = None; - let mut to = None; - let mut gas_price = None; - let mut max_fee_per_gas = None; - let mut max_priority_fee_per_gas = None; - let mut gas = None; - let mut value = None; - let mut data = None; - let mut input = None; - let mut nonce = None; - let mut access_list = None; - let mut transaction_type = None; - - while let Some(key) = map.next_key()? { - match key { - Field::From => { - if from.is_some() { - return Err(de::Error::duplicate_field("from")); - } - from = Some(map.next_value()?); - } - Field::To => { - if to.is_some() { - return Err(de::Error::duplicate_field("to")); - } - to = Some(map.next_value()?); - } - Field::GasPrice => { - if gas_price.is_some() { - return Err(de::Error::duplicate_field("gasPrice")); - } - gas_price = Some(map.next_value()?); - } - Field::MaxFeePerGas => { - if max_fee_per_gas.is_some() { - return Err(de::Error::duplicate_field("maxFeePerGas")); - } - max_fee_per_gas = Some(map.next_value()?); - } - Field::MaxPriorityFeePerGas => { - if max_priority_fee_per_gas.is_some() { - return Err(de::Error::duplicate_field("maxPriorityFeePerGas")); - } - max_priority_fee_per_gas = Some(map.next_value()?); - } - Field::Gas => { - if gas.is_some() { - return Err(de::Error::duplicate_field("gas")); - } - gas = Some(map.next_value()?); - } - Field::Value => { - if value.is_some() { - return Err(de::Error::duplicate_field("value")); - } - value = Some(map.next_value()?); - } - Field::Data => { - if data.is_some() { - return Err(de::Error::duplicate_field("data")); - } - data = Some(map.next_value()?); - } - Field::Input => { - if input.is_some() { - return Err(de::Error::duplicate_field("input")); - } - input = Some(map.next_value()?); - } - Field::Nonce => { - if nonce.is_some() { - return Err(de::Error::duplicate_field("nonce")); - } - nonce = Some(map.next_value()?); - } - Field::AccessList => { - if access_list.is_some() { - return Err(de::Error::duplicate_field("accessList")); - } - access_list = Some(map.next_value()?); - } - Field::Type => { - if transaction_type.is_some() { - return Err(de::Error::duplicate_field("type")); - } - transaction_type = Some(map.next_value()?); - } - } - } - - match (data.as_ref(), input.as_ref()) { - (Some(data), Some(input)) if data != input => { - return Err(de::Error::custom( - "data and input must be equal when both are present", - )) - } - // Assume that the data field is the input field if the data field is not present - // and the input field is present. - (None, Some(_)) => data = input.take(), - _ => {} - } - - Ok(TransactionRequest { - from, - to, - gas_price, - max_fee_per_gas, - max_priority_fee_per_gas, - gas, - value, - data, - input, - nonce, - access_list, - transaction_type, - }) - } - } - - deserializer.deserialize_struct( - "TransactionRequest", - &[ - "from", - "to", - "gasPrice", - "maxFeePerGas", - "maxPriorityFeePerGas", - "gas", - "value", - "data", - "input", - "nonce", - "accessList", - "type", - ], - TransactionRequestVisitor, - ) - } -} #[cfg(test)] mod tests { - use std::str::FromStr; use super::*; use serde_json::json; #[test] - fn test_deserialize_transaction_request() { + fn test_deserialize_with_only_input() { let data = json!({ "from": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", "to": "0x13fe2d1d3665660d22ff9624b7be0551ee1ac91b", @@ -317,58 +138,84 @@ mod tests { "maxPriorityFeePerGas": "0x30", "gas": "0x40", "value": "0x50", - "data": "0x123abc", "input": "0x123abc", "nonce": "0x60", "accessList": [{"address": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", "storageKeys": []}], "type": "0x70" }); - let tr: TransactionRequest = serde_json::from_value(data).unwrap(); + let request: Result = serde_json::from_value(data); + assert!(request.is_ok()); - assert_eq!( - tr.from.unwrap(), - H160::from_str("0x60be2d1d3665660d22ff9624b7be0551ee1ac91b").unwrap() - ); - assert_eq!( - tr.to.unwrap(), - H160::from_str("0x13fe2d1d3665660d22ff9624b7be0551ee1ac91b").unwrap() - ); - assert_eq!(tr.gas_price.unwrap(), U256::from(0x10)); - assert_eq!(tr.max_fee_per_gas.unwrap(), U256::from(0x20)); - assert_eq!(tr.max_priority_fee_per_gas.unwrap(), U256::from(0x30)); - assert_eq!(tr.gas.unwrap(), U256::from(0x40)); - assert_eq!(tr.value.unwrap(), U256::from(0x50)); - assert_eq!(tr.nonce.unwrap(), U256::from(0x60)); - assert_eq!(tr.access_list.unwrap().len(), 1); - assert_eq!(tr.transaction_type.unwrap(), U256::from(0x70)); + let request = request.unwrap(); + assert_eq!(request.data, Some(Bytes::from(vec![0x12, 0x3a, 0xbc]))); } #[test] - fn test_deserialize_transaction_request_data_input_mismatch_error() { + fn test_deserialize_with_only_data() { let data = json!({ - "data": "0xabc2", - "input": "0xdef1", + "from": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", + "to": "0x13fe2d1d3665660d22ff9624b7be0551ee1ac91b", + "gasPrice": "0x10", + "maxFeePerGas": "0x20", + "maxPriorityFeePerGas": "0x30", + "gas": "0x40", + "value": "0x50", + "data": "0x123abc", + "nonce": "0x60", + "accessList": [{"address": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", "storageKeys": []}], + "type": "0x70" }); - let result: Result = serde_json::from_value(data); + let request: Result = serde_json::from_value(data); + assert!(request.is_ok()); - assert!(result.is_err()); - assert_eq!( - result.unwrap_err().to_string(), - "data and input must be equal when both are present" - ); + let request = request.unwrap(); + assert_eq!(request.data, Some(Bytes::from(vec![0x12, 0x3a, 0xbc]))); } #[test] - fn test_deserialize_transaction_request_data_input_equal() { + fn test_deserialize_with_data_and_input_mismatch() { let data = json!({ - "data": "0xabc2", - "input": "0xabc2", + "from": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", + "to": "0x13fe2d1d3665660d22ff9624b7be0551ee1ac91b", + "gasPrice": "0x10", + "maxFeePerGas": "0x20", + "maxPriorityFeePerGas": "0x30", + "gas": "0x40", + "value": "0x50", + "data": "0x123abc", + "input": "0x456def", + "nonce": "0x60", + "accessList": [{"address": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", "storageKeys": []}], + "type": "0x70" + }); + + let request: Result = serde_json::from_value(data); + assert!(request.is_err()); + } + + #[test] + fn test_deserialize_with_data_and_input_equal() { + let data = json!({ + "from": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", + "to": "0x13fe2d1d3665660d22ff9624b7be0551ee1ac91b", + "gasPrice": "0x10", + "maxFeePerGas": "0x20", + "maxPriorityFeePerGas": "0x30", + "gas": "0x40", + "value": "0x50", + "data": "0x123abc", + "input": "0x123abc", + "nonce": "0x60", + "accessList": [{"address": "0x60be2d1d3665660d22ff9624b7be0551ee1ac91b", "storageKeys": []}], + "type": "0x70" }); - let result: Result = serde_json::from_value(data); + let request: Result = serde_json::from_value(data); + assert!(request.is_ok()); - assert!(result.is_ok()); + let request = request.unwrap(); + assert_eq!(request.data, Some(Bytes::from(vec![0x12, 0x3a, 0xbc]))); } } From 71f4e9153a3d989382daa6c0c4dbb3a87f1c80a2 Mon Sep 17 00:00:00 2001 From: Ahmad Kaouk Date: Tue, 4 Jul 2023 14:14:28 +0200 Subject: [PATCH 09/11] fix formatting --- client/rpc-core/src/types/call_request.rs | 22 ++++++++++++------- .../rpc-core/src/types/transaction_request.rs | 1 - 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/client/rpc-core/src/types/call_request.rs b/client/rpc-core/src/types/call_request.rs index 53766d4d8e..4b62dff7d5 100644 --- a/client/rpc-core/src/types/call_request.rs +++ b/client/rpc-core/src/types/call_request.rs @@ -30,15 +30,21 @@ pub(crate) struct CallOrInputData { input: Option, } -pub(crate) fn deserialize_data_or_input<'d, D: Deserializer<'d>>(d: D) -> Result, D::Error> { - let CallOrInputData { data, input } = CallOrInputData::deserialize(d)?; +pub(crate) fn deserialize_data_or_input<'d, D: Deserializer<'d>>( + d: D, +) -> Result, D::Error> { + let CallOrInputData { data, input } = CallOrInputData::deserialize(d)?; match (&data, &input) { - (Some(data), Some(input)) => if data == input { - Ok(Some(data.clone())) - } else { - Err(D::Error::custom("Ambiguous value for `data` and `input`".to_string())) - }, - (_, _) => Ok(data.or(input)) + (Some(data), Some(input)) => { + if data == input { + Ok(Some(data.clone())) + } else { + Err(D::Error::custom( + "Ambiguous value for `data` and `input`".to_string(), + )) + } + } + (_, _) => Ok(data.or(input)), } } diff --git a/client/rpc-core/src/types/transaction_request.rs b/client/rpc-core/src/types/transaction_request.rs index 1d117fb2b4..7de0564b62 100644 --- a/client/rpc-core/src/types/transaction_request.rs +++ b/client/rpc-core/src/types/transaction_request.rs @@ -121,7 +121,6 @@ impl From for Option { } } - #[cfg(test)] mod tests { From c6eab2b97aadff9c3920462a6baf9d9a7d51d919 Mon Sep 17 00:00:00 2001 From: Ahmad Kaouk Date: Tue, 4 Jul 2023 14:23:05 +0200 Subject: [PATCH 10/11] update functional test --- ts-tests/tests/test-execute.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ts-tests/tests/test-execute.ts b/ts-tests/tests/test-execute.ts index 4a62f9caa4..671cee5a5d 100644 --- a/ts-tests/tests/test-execute.ts +++ b/ts-tests/tests/test-execute.ts @@ -261,6 +261,6 @@ describeWithFrontier("Frontier RPC (RPC execution)", (context) => { input: "0x12345678", }, ]); - expect((result as any).error.message).to.match(/^data and input must be equal when both are present/); + expect((result as any).error.message).to.match(/^Ambiguous value for `data` and `input`/); }); }); From 3ad4bd774de5bc1961c733984e04eb72d1abb21f Mon Sep 17 00:00:00 2001 From: Ahmad Kaouk Date: Wed, 5 Jul 2023 08:02:30 +0200 Subject: [PATCH 11/11] refactor code and fix formatting --- client/rpc-core/src/types/call_request.rs | 30 ++----------------- client/rpc-core/src/types/mod.rs | 28 +++++++++++++++++ .../rpc-core/src/types/transaction_request.rs | 6 ++-- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/client/rpc-core/src/types/call_request.rs b/client/rpc-core/src/types/call_request.rs index 4b62dff7d5..b699e0aea1 100644 --- a/client/rpc-core/src/types/call_request.rs +++ b/client/rpc-core/src/types/call_request.rs @@ -18,35 +18,10 @@ use std::collections::BTreeMap; +use crate::types::{deserialize_data_or_input, Bytes}; use ethereum::AccessListItem; use ethereum_types::{H160, H256, U256}; -use serde::{de::Error, Deserialize, Deserializer}; - -use crate::types::Bytes; - -#[derive(Clone, Debug, Default, Eq, PartialEq, Deserialize)] -pub(crate) struct CallOrInputData { - data: Option, - input: Option, -} - -pub(crate) fn deserialize_data_or_input<'d, D: Deserializer<'d>>( - d: D, -) -> Result, D::Error> { - let CallOrInputData { data, input } = CallOrInputData::deserialize(d)?; - match (&data, &input) { - (Some(data), Some(input)) => { - if data == input { - Ok(Some(data.clone())) - } else { - Err(D::Error::custom( - "Ambiguous value for `data` and `input`".to_string(), - )) - } - } - (_, _) => Ok(data.or(input)), - } -} +use serde::Deserialize; /// Call request #[derive(Clone, Debug, Default, Eq, PartialEq, Deserialize)] @@ -100,7 +75,6 @@ pub struct CallStateOverride { #[cfg(test)] mod tests { - use super::*; use serde_json::json; diff --git a/client/rpc-core/src/types/mod.rs b/client/rpc-core/src/types/mod.rs index 4384415428..1744014041 100644 --- a/client/rpc-core/src/types/mod.rs +++ b/client/rpc-core/src/types/mod.rs @@ -35,6 +35,8 @@ mod work; pub mod pubsub; +use serde::{de::Error, Deserialize, Deserializer}; + pub use self::{ account_info::{AccountInfo, EthAccount, ExtAccountInfo, RecoveredAccount, StorageProof}, block::{Block, BlockTransactions, Header, Rich, RichBlock, RichHeader}, @@ -57,3 +59,29 @@ pub use self::{ transaction_request::{TransactionMessage, TransactionRequest}, work::Work, }; + +#[derive(Clone, Debug, Default, Eq, PartialEq, Deserialize)] +pub(crate) struct CallOrInputData { + data: Option, + input: Option, +} + +/// Function to deserialize `data` and `input` within `TransactionRequest` and `CallRequest`. +/// It verifies that if both `data` and `input` are provided, they must be identical. +pub(crate) fn deserialize_data_or_input<'d, D: Deserializer<'d>>( + d: D, +) -> Result, D::Error> { + let CallOrInputData { data, input } = CallOrInputData::deserialize(d)?; + match (&data, &input) { + (Some(data), Some(input)) => { + if data == input { + Ok(Some(data.clone())) + } else { + Err(D::Error::custom( + "Ambiguous value for `data` and `input`".to_string(), + )) + } + } + (_, _) => Ok(data.or(input)), + } +} diff --git a/client/rpc-core/src/types/transaction_request.rs b/client/rpc-core/src/types/transaction_request.rs index 7de0564b62..f94d8376f9 100644 --- a/client/rpc-core/src/types/transaction_request.rs +++ b/client/rpc-core/src/types/transaction_request.rs @@ -17,15 +17,14 @@ // along with this program. If not, see . //! `TransactionRequest` type + use ethereum::{ AccessListItem, EIP1559TransactionMessage, EIP2930TransactionMessage, LegacyTransactionMessage, }; use ethereum_types::{H160, U256}; use serde::{Deserialize, Serialize}; -use crate::types::Bytes; - -use super::call_request::deserialize_data_or_input; +use crate::types::{deserialize_data_or_input, Bytes}; pub enum TransactionMessage { Legacy(LegacyTransactionMessage), @@ -123,7 +122,6 @@ impl From for Option { #[cfg(test)] mod tests { - use super::*; use serde_json::json;