Skip to content

Commit

Permalink
Handling data and input fields in TransactionRequest and `CallR…
Browse files Browse the repository at this point in the history
…equest` (#1096)

* Update deserializing data and input fields in TransactionRequest

* Custom deserializing for data and input in CallRequest

* Manually implement Deserialize for TransactionRequest and CallRequest

* Add some unit test for the custom deserialization

* add functional tests

* fix formatting

* deserializer for aliased fields

* custom deserializer for data and input fields in TransactionRequest

* fix formatting

* update functional test

* refactor code and fix formatting

---------

Co-authored-by: tgmichel <[email protected]>
  • Loading branch information
ahmadkaouk and tgmichel authored Jul 18, 2023
1 parent 9f38ec4 commit bd5bd37
Show file tree
Hide file tree
Showing 4 changed files with 251 additions and 5 deletions.
102 changes: 99 additions & 3 deletions client/rpc-core/src/types/call_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@

use std::collections::BTreeMap;

use crate::types::{deserialize_data_or_input, Bytes};
use ethereum::AccessListItem;
use ethereum_types::{H160, H256, U256};
use serde::Deserialize;

use crate::types::Bytes;

/// Call request
#[derive(Clone, Debug, Default, Eq, PartialEq, Deserialize)]
#[serde(deny_unknown_fields)]
Expand All @@ -44,7 +43,7 @@ pub struct CallRequest {
/// Value
pub value: Option<U256>,
/// Data
#[serde(alias = "input")]
#[serde(deserialize_with = "deserialize_data_or_input", flatten)]
pub data: Option<Bytes>,
/// Nonce
pub nonce: Option<U256>,
Expand Down Expand Up @@ -73,3 +72,100 @@ pub struct CallStateOverride {
/// executing the call.
pub state_diff: Option<BTreeMap<H256, H256>>,
}

#[cfg(test)]
mod tests {
use super::*;
use serde_json::json;

#[test]
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<CallRequest, _> = 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<CallRequest, _> = 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<CallRequest, _> = 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 request: Result<CallRequest, _> = serde_json::from_value(data);
assert!(request.is_ok());

let request = request.unwrap();
assert_eq!(request.data, Some(Bytes::from(vec![0x12, 0x3a, 0xbc])));
}
}
28 changes: 28 additions & 0 deletions client/rpc-core/src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,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},
Expand All @@ -59,3 +61,29 @@ pub use self::{
txpool::{Get, Summary, TransactionMap, TxPoolResult, TxPoolTransaction},
work::Work,
};

#[derive(Clone, Debug, Default, Eq, PartialEq, Deserialize)]
pub(crate) struct CallOrInputData {
data: Option<Bytes>,
input: Option<Bytes>,
}

/// 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<Option<Bytes>, 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)),
}
}
101 changes: 99 additions & 2 deletions client/rpc-core/src/types/transaction_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use ethereum::{
use ethereum_types::{H160, U256};
use serde::{Deserialize, Serialize};

use crate::types::Bytes;
use crate::types::{deserialize_data_or_input, Bytes};

pub enum TransactionMessage {
Legacy(LegacyTransactionMessage),
Expand Down Expand Up @@ -55,7 +55,7 @@ pub struct TransactionRequest {
/// Value of transaction in wei
pub value: Option<U256>,
/// Additional data sent with transaction
#[serde(alias = "input")]
#[serde(deserialize_with = "deserialize_data_or_input", flatten)]
pub data: Option<Bytes>,
/// Transaction's nonce
pub nonce: Option<U256>,
Expand Down Expand Up @@ -119,3 +119,100 @@ impl From<TransactionRequest> for Option<TransactionMessage> {
}
}
}

#[cfg(test)]
mod tests {
use super::*;
use serde_json::json;

#[test]
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<TransactionRequest, _> = 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<TransactionRequest, _> = 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<TransactionRequest, _> = 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 request: Result<TransactionRequest, _> = serde_json::from_value(data);
assert!(request.is_ok());

let request = request.unwrap();
assert_eq!(request.data, Some(Bytes::from(vec![0x12, 0x3a, 0xbc])));
}
}
25 changes: 25 additions & 0 deletions ts-tests/tests/test-execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(/^Ambiguous value for `data` and `input`/);
});
});

0 comments on commit bd5bd37

Please sign in to comment.