From dcb69ba35322e895fa5ca4d2b9e12c91fd0df15f Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Mon, 25 Nov 2019 12:03:50 +0100 Subject: [PATCH] [ethcore]: apply filter when `PendingSet::AlwaysQueue` in `ready_transactions_filtered` (#11227) * [ethcore]: apply filter when `from_queue` In `ready_transactions_filtered` the filter were never applied when the options PendingSet::AlwaysQueue` was configured which this fixes It also adds a two tests for it * [ethcore test-helpers]: stray printlns * docs(ethcore filter options): more generic desc * tests(ethcore miner): simply filter tests * [ethcore filter_options]: fix nits * doc: nit Co-Authored-By: David * doc: nit Co-Authored-By: David * doc: nit Co-Authored-By: David * doc: nit Co-Authored-By: David * doc: nit Co-Authored-By: David * doc(miner filter): simplify documentation * [rpc]: make tests compile --- ethcore/src/miner/filter_options.rs | 1556 +++++++++++---------- ethcore/src/miner/miner.rs | 169 ++- rpc/src/v1/tests/helpers/miner_service.rs | 17 +- 3 files changed, 922 insertions(+), 820 deletions(-) diff --git a/ethcore/src/miner/filter_options.rs b/ethcore/src/miner/filter_options.rs index ae1a206b0..798006da8 100644 --- a/ethcore/src/miner/filter_options.rs +++ b/ethcore/src/miner/filter_options.rs @@ -14,39 +14,81 @@ // You should have received a copy of the GNU General Public License // along with Parity Ethereum. If not, see . -use ethereum_types::{Address, U256}; -use serde::de::{Deserialize, Deserializer, Error, MapAccess, Visitor}; use std::fmt; use std::marker::PhantomData; -/// This structure provides filtering options for the pending transactions RPC API call +use ethereum_types::{Address, U256}; +use serde::de::{Deserialize, Deserializer, Error, MapAccess, Visitor}; +use types::transaction::SignedTransaction; + +/// Filtering options for the pending transactions +/// May be used for filtering transactions based on gas, gas price, value and/or nonce. +// NOTE: the fields are only `pub` because they are needed for tests #[derive(Debug, Clone, Eq, PartialEq)] pub struct FilterOptions { - /// Contains the operator to filter the from value of the transaction - pub from: FilterOperator
, - /// Contains the operator to filter the to value of the transaction - pub to: FilterOperator>, - /// Contains the operator to filter the gas value of the transaction - pub gas: FilterOperator, - /// Contains the operator to filter the gas price value of the transaction - pub gas_price: FilterOperator, - /// Contains the operator to filter the transaction value - pub value: FilterOperator, - /// Contains the operator to filter the nonce value of the transaction - pub nonce: FilterOperator, + /// Filter based on the `sender` of the transaction. + pub from: FilterOperator
, + /// Filter based on `receiver` of the transaction. + pub to: FilterOperator>, + /// Filter based on `gas` of the transaction. + pub gas: FilterOperator, + /// Filter based on `gas price` of the transaction. + pub gas_price: FilterOperator, + /// Filter based on `value` of the transaction. + pub value: FilterOperator, + /// Filter based on `nonce` of the transaction. + pub nonce: FilterOperator, +} + +impl FilterOptions { + fn sender_matcher(filter: &FilterOperator
, candidate: &Address) -> bool { + match filter { + FilterOperator::Eq(address) => candidate == address, + FilterOperator::Any => true, + // Handled during deserialization + _ => unreachable!(), + } + } + + fn receiver_matcher(filter: &FilterOperator>, candidate: &Option
) -> bool { + match filter { + FilterOperator::Eq(address) => candidate == address, + FilterOperator::Any => true, + // Handled during deserialization + _ => unreachable!(), + } + } + fn value_matcher(filter: &FilterOperator, tx_value: &U256) -> bool { + match filter { + FilterOperator::Eq(ref value) => tx_value == value, + FilterOperator::GreaterThan(ref value) => tx_value > value, + FilterOperator::LessThan(ref value) => tx_value < value, + FilterOperator::Any => true, + } + } + + /// Determines whether a transaction passes the configured filter + pub fn matches(&self, tx: &SignedTransaction) -> bool { + Self::sender_matcher(&self.from, &tx.sender()) && + Self::receiver_matcher(&self.to, &tx.receiver()) && + Self::value_matcher(&self.gas, &tx.gas) && + Self::value_matcher(&self.gas_price, &tx.gas_price) && + Self::value_matcher(&self.value, &tx.value) && + Self::value_matcher(&self.nonce, &tx.nonce) + } } impl Default for FilterOptions { - fn default() -> Self { - FilterOptions { - from: FilterOperator::Any, - to: FilterOperator::Any, - gas: FilterOperator::Any, - gas_price: FilterOperator::Any, - value: FilterOperator::Any, - nonce: FilterOperator::Any, - } - } + fn default() -> Self { + FilterOptions { + from: FilterOperator::Any, + to: FilterOperator::Any, + gas: FilterOperator::Any, + gas_price: FilterOperator::Any, + value: FilterOperator::Any, + nonce: FilterOperator::Any, + } + } } /// The highly generic use of implementing Deserialize for FilterOperator @@ -54,8 +96,10 @@ impl Default for FilterOptions { /// gets returned explicitly. Therefore this Wrapper will be used for /// deserialization, directly identifying the contract creation. enum Wrapper { - O(FilterOperator), - CC, // Contract Creation + /// FilterOperations + O(FilterOperator), + /// Contract Creation + CC, } /// Available operators for filtering options. @@ -63,10 +107,14 @@ enum Wrapper { /// The `to` filter only accepts Any, Eq(Address) and Eq(None) for contract creation. #[derive(Debug, Clone, Eq, PartialEq)] pub enum FilterOperator { - Any, - Eq(T), - GreaterThan(T), - LessThan(T), + /// Any (no filter) + Any, + /// Equal + Eq(T), + /// Greather than + GreaterThan(T), + /// Less than + LessThan(T), } /// Since there are multiple operators which are not supported equally by all filters, @@ -78,792 +126,792 @@ pub enum FilterOperator { /// The `to` filter validates with `validate_from` /// All other filters such as gas and price validate with `validate_value` trait Validate<'de, T, M: MapAccess<'de>> { - fn validate_from(&mut self) -> Result, M::Error>; - fn validate_to(&mut self) -> Result>, M::Error>; - fn validate_value(&mut self) -> Result, M::Error>; + fn validate_from(&mut self) -> Result, M::Error>; + fn validate_to(&mut self) -> Result>, M::Error>; + fn validate_value(&mut self) -> Result, M::Error>; } -impl<'de, T, M> Validate<'de, T, M> for M - where T: Deserialize<'de>, M: MapAccess<'de> +impl<'de, T, M> Validate<'de, T, M> for M +where + T: Deserialize<'de>, M: MapAccess<'de> { - fn validate_from(&mut self) -> Result, M::Error> { - use self::Wrapper as W; - use self::FilterOperator::*; - let wrapper = self.next_value()?; - match wrapper { - W::O(val) => { - match val { - Any | Eq(_) => Ok(val), - _ => { - Err(M::Error::custom( - "the `from` filter only supports the `eq` operator", - )) - } - } - }, - W::CC => { - Err(M::Error::custom( - "the `from` filter only supports the `eq` operator", - )) - } - } - } - fn validate_to(&mut self) -> Result>, M::Error> { - use self::Wrapper as W; - use self::FilterOperator::*; - let wrapper = self.next_value()?; - match wrapper { - W::O(val) => { - match val { - Any => Ok(Any), - Eq(address) => Ok(Eq(Some(address))), - _ => { - Err(M::Error::custom( - "the `to` filter only supports the `eq` or `action` operator", - )) - } - } - }, - W::CC => Ok(FilterOperator::Eq(None)), - } - } - fn validate_value(&mut self) -> Result, M::Error> { - use self::Wrapper as W; - let wrapper = self.next_value()?; - match wrapper { - W::O(val) => Ok(val), - W::CC => { - Err(M::Error::custom( - "the operator `action` is only supported by the `to` filter", - )) - } - } - } + fn validate_from(&mut self) -> Result, M::Error> { + use self::Wrapper as W; + use self::FilterOperator::*; + let wrapper = self.next_value()?; + match wrapper { + W::O(val) => { + match val { + Any | Eq(_) => Ok(val), + _ => { + Err(M::Error::custom( + "the `from` filter only supports the `eq` operator", + )) + } + } + }, + W::CC => { + Err(M::Error::custom( + "the `from` filter only supports the `eq` operator", + )) + } + } + } + fn validate_to(&mut self) -> Result>, M::Error> { + use self::Wrapper as W; + use self::FilterOperator::*; + let wrapper = self.next_value()?; + match wrapper { + W::O(val) => { + match val { + Any => Ok(Any), + Eq(address) => Ok(Eq(Some(address))), + _ => { + Err(M::Error::custom( + "the `to` filter only supports the `eq` or `action` operator", + )) + } + } + }, + W::CC => Ok(FilterOperator::Eq(None)), + } + } + fn validate_value(&mut self) -> Result, M::Error> { + use self::Wrapper as W; + let wrapper = self.next_value()?; + match wrapper { + W::O(val) => Ok(val), + W::CC => { + Err(M::Error::custom( + "the operator `action` is only supported by the `to` filter", + )) + } + } + } } impl<'de> Deserialize<'de> for FilterOptions { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - struct FilterOptionsVisitor; - impl<'de> Visitor<'de> for FilterOptionsVisitor { - type Value = FilterOptions; + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct FilterOptionsVisitor; + impl<'de> Visitor<'de> for FilterOptionsVisitor { + type Value = FilterOptions; - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - // "This Visitor expects to receive ..." - formatter.write_str("a map with one valid filter such as `from`, `to`, `gas`, `gas_price`, `value` or `nonce`") - } + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + // "This Visitor expects to receive ..." + formatter.write_str("a map with one valid filter such as `from`, `to`, `gas`, `gas_price`, `value` or `nonce`") + } - fn visit_map(self, mut map: M) -> Result - where - M: MapAccess<'de>, - { - let mut filter = FilterOptions::default(); - while let Some(key) = map.next_key::()? { - match key.as_str() { - "from" => { - filter.from = map.validate_from()?; - }, - "to" => { - // Compiler cannot infer type, so set one (nothing specific for this method) - filter.to = Validate::<(), _>::validate_to(&mut map)?; - }, - "gas" => { - filter.gas = map.validate_value()?; - }, - "gas_price" => { - filter.gas_price = map.validate_value()?; - }, - "value" => { - filter.value = map.validate_value()?; - }, - "nonce" => { - filter.nonce = map.validate_value()?; - }, - unknown => { - return Err(M::Error::unknown_field( - unknown, - &["from", "to", "gas", "gas_price", "value", "nonce"], - )) - } - } - } + fn visit_map(self, mut map: M) -> Result + where + M: MapAccess<'de>, + { + let mut filter = FilterOptions::default(); + while let Some(key) = map.next_key::()? { + match key.as_str() { + "from" => { + filter.from = map.validate_from()?; + }, + "to" => { + // Compiler cannot infer type, so set one (nothing specific for this method) + filter.to = Validate::<(), _>::validate_to(&mut map)?; + }, + "gas" => { + filter.gas = map.validate_value()?; + }, + "gas_price" => { + filter.gas_price = map.validate_value()?; + }, + "value" => { + filter.value = map.validate_value()?; + }, + "nonce" => { + filter.nonce = map.validate_value()?; + }, + unknown => { + return Err(M::Error::unknown_field( + unknown, + &["from", "to", "gas", "gas_price", "value", "nonce"], + )) + } + } + } + Ok(filter) + } + } - Ok(filter) - } - } + impl<'de, T: Deserialize<'de>> Deserialize<'de> for Wrapper { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct WrapperVisitor { + data: PhantomData, + }; + impl<'de, T: Deserialize<'de>> Visitor<'de> for WrapperVisitor { + type Value = Wrapper; - impl<'de, T: Deserialize<'de>> Deserialize<'de> for Wrapper { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - struct WrapperVisitor { - data: PhantomData, - }; - impl<'de, T: Deserialize<'de>> Visitor<'de> for WrapperVisitor { - type Value = Wrapper; + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + // "This Visitor expects to receive ..." + formatter.write_str( + "a map with one valid operator such as `eq`, `gt` or `lt`. \ + The to filter can also contain `action`", + ) + } - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - // "This Visitor expects to receive ..." - formatter.write_str( - "a map with one valid operator such as `eq`, `gt` or `lt`. \ - The to filter can also contain `action`", - ) - } + fn visit_map(self, mut map: M) -> Result + where + M: MapAccess<'de>, + { + use self::Wrapper as W; + let mut counter = 0; + let mut f_op = Wrapper::O(FilterOperator::Any); - fn visit_map(self, mut map: M) -> Result - where - M: MapAccess<'de>, - { - use self::Wrapper as W; - let mut counter = 0; - let mut f_op = Wrapper::O(FilterOperator::Any); + while let Some(key) = map.next_key::()? { + match key.as_str() { + "eq" => f_op = W::O(FilterOperator::Eq(map.next_value()?)), + "gt" => f_op = W::O(FilterOperator::GreaterThan(map.next_value()?)), + "lt" => f_op = W::O(FilterOperator::LessThan(map.next_value()?)), + "action" => { + match map.next_value()? { + "contract_creation" => { + f_op = W::CC; + }, + _ => { + return Err(M::Error::custom( + "`action` only supports the value `contract_creation`", + )) + } + } + } + unknown => { + // skip mentioning `action` since it's a special/rare + // case and might confuse the usage with other filters. + return Err(M::Error::unknown_field(unknown, &["eq", "gt", "lt"])); + } + } - while let Some(key) = map.next_key::()? { - match key.as_str() { - "eq" => f_op = W::O(FilterOperator::Eq(map.next_value()?)), - "gt" => f_op = W::O(FilterOperator::GreaterThan(map.next_value()?)), - "lt" => f_op = W::O(FilterOperator::LessThan(map.next_value()?)), - "action" => { - match map.next_value()? { - "contract_creation" => { - f_op = W::CC; - }, - _ => { - return Err(M::Error::custom( - "`action` only supports the value `contract_creation`", - )) - } - } - } - unknown => { - // skip mentioning `action` since it's a special/rare - // case and might confuse the usage with other filters. - return Err(M::Error::unknown_field(unknown, &["eq", "gt", "lt"])); - } - } + counter += 1; + } - counter += 1; - } + // Good practices ensured: only one operator per filter field is allowed. + // In case there is more than just one operator, this method must still process + // all of them, otherwise serde returns an error mentioning a trailing comma issue + // (even on valid JSON), which is misleading to the user of this software. + if counter > 1 { + return Err(M::Error::custom( + "only one operator per filter type allowed", + )); + } - // Good practices ensured: only one operator per filter field is allowed. - // In case there is more than just one operator, this method must still process - // all of them, otherwise serde returns an error mentioning a trailing comma issue - // (even on valid JSON), which is misleading to the user of this software. - if counter > 1 { - return Err(M::Error::custom( - "only one operator per filter type allowed", - )); - } + Ok(f_op) + } + } - Ok(f_op) - } - } + deserializer.deserialize_map(WrapperVisitor { data: PhantomData }) + } + } - deserializer.deserialize_map(WrapperVisitor { data: PhantomData }) - } - } - - deserializer.deserialize_map(FilterOptionsVisitor) - } + deserializer.deserialize_map(FilterOptionsVisitor) + } } #[cfg(test)] mod tests { - use ethereum_types::{Address, U256}; - use serde_json; - use super::*; + use ethereum_types::{Address, U256}; + use serde_json; + use super::*; use std::str::FromStr; - #[test] - fn valid_defaults() { - let default = FilterOptions::default(); - assert_eq!(default.from, FilterOperator::Any); - assert_eq!(default.to, FilterOperator::Any); - assert_eq!(default.gas, FilterOperator::Any); - assert_eq!(default.gas_price, FilterOperator::Any); - assert_eq!(default.value, FilterOperator::Any); - assert_eq!(default.nonce, FilterOperator::Any); + #[test] + fn valid_defaults() { + let default = FilterOptions::default(); + assert_eq!(default.from, FilterOperator::Any); + assert_eq!(default.to, FilterOperator::Any); + assert_eq!(default.gas, FilterOperator::Any); + assert_eq!(default.gas_price, FilterOperator::Any); + assert_eq!(default.value, FilterOperator::Any); + assert_eq!(default.nonce, FilterOperator::Any); - let json = r#"{}"#; - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, default); - } + let json = r#"{}"#; + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, default); + } - #[test] - fn valid_full_deserialization() { - let json = r#" - { - "from": { - "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f" - }, - "to": { - "eq": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" - }, - "gas": { - "eq": "0x493e0" - }, - "gas_price": { - "eq": "0x12a05f200" - }, - "value": { - "eq": "0x0" - }, - "nonce": { - "eq": "0x577" - } - } - "#; + #[test] + fn valid_full_deserialization() { + let json = r#" + { + "from": { + "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f" + }, + "to": { + "eq": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" + }, + "gas": { + "eq": "0x493e0" + }, + "gas_price": { + "eq": "0x12a05f200" + }, + "value": { + "eq": "0x0" + }, + "nonce": { + "eq": "0x577" + } + } + "#; - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - from: FilterOperator::Eq(Address::from_str("5f3dffcf347944d3739b0805c934d86c8621997f").unwrap()), - to: FilterOperator::Eq(Some(Address::from_str("e8b2d01ffa0a15736b2370b6e5064f9702c891b6").unwrap())), - gas: FilterOperator::Eq(U256::from(300_000)), - gas_price: FilterOperator::Eq(U256::from(5_000_000_000 as i64)), - value: FilterOperator::Eq(U256::from(0)), - nonce: FilterOperator::Eq(U256::from(1399)), - }) - } + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + from: FilterOperator::Eq(Address::from_str("5f3dffcf347944d3739b0805c934d86c8621997f").unwrap()), + to: FilterOperator::Eq(Some(Address::from_str("e8b2d01ffa0a15736b2370b6e5064f9702c891b6").unwrap())), + gas: FilterOperator::Eq(U256::from(300_000)), + gas_price: FilterOperator::Eq(U256::from(5_000_000_000 as i64)), + value: FilterOperator::Eq(U256::from(0)), + nonce: FilterOperator::Eq(U256::from(1399)), + }) + } - #[test] - fn invalid_full_deserialization() { - // Invalid filter type `zyx` - let json = r#" - { - "from": { - "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f" - }, - "to": { - "eq": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" - }, - "zyx": { - "eq": "0x493e0" - }, - "gas_price": { - "eq": "0x12a05f200" - }, - "value": { - "eq": "0x0" - }, - "nonce": { - "eq": "0x577" - } - } - "#; + #[test] + fn invalid_full_deserialization() { + // Invalid filter type `zyx` + let json = r#" + { + "from": { + "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f" + }, + "to": { + "eq": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" + }, + "zyx": { + "eq": "0x493e0" + }, + "gas_price": { + "eq": "0x12a05f200" + }, + "value": { + "eq": "0x0" + }, + "nonce": { + "eq": "0x577" + } + } + "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()) - } + let res = serde_json::from_str::(json); + assert!(res.is_err()) + } - #[test] - fn valid_from_operators() { - // Only one valid operator for from - let json = r#" - { - "from": { - "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f" - } - } - "#; - let default = FilterOptions::default(); - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - from: FilterOperator::Eq(Address::from_str("5f3dffcf347944d3739b0805c934d86c8621997f").unwrap()), - ..default - }); - } + #[test] + fn valid_from_operators() { + // Only one valid operator for from + let json = r#" + { + "from": { + "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + from: FilterOperator::Eq(Address::from_str("5f3dffcf347944d3739b0805c934d86c8621997f").unwrap()), + ..default + }); + } - #[test] - fn invalid_from_operators() { - // Multiple operators are invalid - let json = r#" - { - "from": { - "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f", - "lt": "0x407d73d8a49eeb85d32cf465507dd71d507100c1" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + #[test] + fn invalid_from_operators() { + // Multiple operators are invalid + let json = r#" + { + "from": { + "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f", + "lt": "0x407d73d8a49eeb85d32cf465507dd71d507100c1" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Gt - let json = r#" - { - "from": { - "gt": "0x5f3dffcf347944d3739b0805c934d86c8621997f" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + // Gt + let json = r#" + { + "from": { + "gt": "0x5f3dffcf347944d3739b0805c934d86c8621997f" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Lt - let json = r#" - { - "from": { - "lt": "0x5f3dffcf347944d3739b0805c934d86c8621997f" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + // Lt + let json = r#" + { + "from": { + "lt": "0x5f3dffcf347944d3739b0805c934d86c8621997f" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Action - let json = r#" - { - "from": { - "action": "contract_creation" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + // Action + let json = r#" + { + "from": { + "action": "contract_creation" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Unknown operator - let json = r#" - { - "from": { - "abc": "0x0" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); - } + // Unknown operator + let json = r#" + { + "from": { + "abc": "0x0" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + } - #[test] - fn valid_to_operators() { - // Only two valid operator for to - // Eq - let json = r#" - { - "to": { - "eq": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" - } - } - "#; - let default = FilterOptions::default(); - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - to: FilterOperator::Eq(Some(Address::from_str("e8b2d01ffa0a15736b2370b6e5064f9702c891b6").unwrap())), - ..default.clone() - }); + #[test] + fn valid_to_operators() { + // Only two valid operator for to + // Eq + let json = r#" + { + "to": { + "eq": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + to: FilterOperator::Eq(Some(Address::from_str("e8b2d01ffa0a15736b2370b6e5064f9702c891b6").unwrap())), + ..default.clone() + }); - // Action - let json = r#" - { - "to": { - "action": "contract_creation" - } - } - "#; - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - to: FilterOperator::Eq(None), - ..default - }); - } + // Action + let json = r#" + { + "to": { + "action": "contract_creation" + } + } + "#; + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + to: FilterOperator::Eq(None), + ..default + }); + } - #[test] - fn invalid_to_operators() { - // Multiple operators are invalid - let json = r#" - { - "to": { - "eq": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6", - "action": "contract_creation" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + #[test] + fn invalid_to_operators() { + // Multiple operators are invalid + let json = r#" + { + "to": { + "eq": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6", + "action": "contract_creation" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Gt - let json = r#" - { - "to": { - "gt": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + // Gt + let json = r#" + { + "to": { + "gt": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Lt - let json = r#" - { - "to": { - "lt": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + // Lt + let json = r#" + { + "to": { + "lt": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Action (invalid value, must be "contract_creation") - let json = r#" - { - "to": { - "action": "some_invalid_value" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + // Action (invalid value, must be "contract_creation") + let json = r#" + { + "to": { + "action": "some_invalid_value" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Unknown operator - let json = r#" - { - "to": { - "abc": "0x0" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); - } + // Unknown operator + let json = r#" + { + "to": { + "abc": "0x0" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + } - #[test] - fn valid_gas_operators() { - // Eq - let json = r#" - { - "gas": { - "eq": "0x493e0" - } - } - "#; - let default = FilterOptions::default(); - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - gas: FilterOperator::Eq(U256::from(300_000)), - ..default.clone() - }); + #[test] + fn valid_gas_operators() { + // Eq + let json = r#" + { + "gas": { + "eq": "0x493e0" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + gas: FilterOperator::Eq(U256::from(300_000)), + ..default.clone() + }); - // Gt - let json = r#" - { - "gas": { - "gt": "0x493e0" - } - } - "#; - let default = FilterOptions::default(); - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - gas: FilterOperator::GreaterThan(U256::from(300_000)), - ..default.clone() - }); + // Gt + let json = r#" + { + "gas": { + "gt": "0x493e0" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + gas: FilterOperator::GreaterThan(U256::from(300_000)), + ..default.clone() + }); - // Lt - let json = r#" - { - "gas": { - "lt": "0x493e0" - } - } - "#; - let default = FilterOptions::default(); - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - gas: FilterOperator::LessThan(U256::from(300_000)), - ..default - }); - } + // Lt + let json = r#" + { + "gas": { + "lt": "0x493e0" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + gas: FilterOperator::LessThan(U256::from(300_000)), + ..default + }); + } - #[test] - fn invalid_gas_operators() { - // Multiple operators are invalid - let json = r#" - { - "gas": { - "eq": "0x493e0", - "lt": "0x493e0" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + #[test] + fn invalid_gas_operators() { + // Multiple operators are invalid + let json = r#" + { + "gas": { + "eq": "0x493e0", + "lt": "0x493e0" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Action - let json = r#" - { - "gas": { - "action": "contract_creation" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + // Action + let json = r#" + { + "gas": { + "action": "contract_creation" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Unknown operator - let json = r#" - { - "gas": { - "abc": "0x0" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); - } + // Unknown operator + let json = r#" + { + "gas": { + "abc": "0x0" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + } - #[test] - fn valid_gas_price_operators() { - // Eq - let json = r#" - { - "gas_price": { - "eq": "0x12a05f200" - } - } - "#; - let default = FilterOptions::default(); - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - gas_price: FilterOperator::Eq(U256::from(5_000_000_000 as i64)), - ..default.clone() - }); + #[test] + fn valid_gas_price_operators() { + // Eq + let json = r#" + { + "gas_price": { + "eq": "0x12a05f200" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + gas_price: FilterOperator::Eq(U256::from(5_000_000_000 as i64)), + ..default.clone() + }); - // Gt - let json = r#" - { - "gas_price": { - "gt": "0x12a05f200" - } - } - "#; - let default = FilterOptions::default(); - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - gas_price: FilterOperator::GreaterThan(U256::from(5_000_000_000 as i64)), - ..default.clone() - }); + // Gt + let json = r#" + { + "gas_price": { + "gt": "0x12a05f200" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + gas_price: FilterOperator::GreaterThan(U256::from(5_000_000_000 as i64)), + ..default.clone() + }); - // Lt - let json = r#" - { - "gas_price": { - "lt": "0x12a05f200" - } - } - "#; - let default = FilterOptions::default(); - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - gas_price: FilterOperator::LessThan(U256::from(5_000_000_000 as i64)), - ..default - }); - } + // Lt + let json = r#" + { + "gas_price": { + "lt": "0x12a05f200" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + gas_price: FilterOperator::LessThan(U256::from(5_000_000_000 as i64)), + ..default + }); + } - #[test] - fn invalid_gas_price_operators() { - // Multiple operators are invalid - let json = r#" - { - "gas_price": { - "eq": "0x12a05f200", - "lt": "0x12a05f200" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + #[test] + fn invalid_gas_price_operators() { + // Multiple operators are invalid + let json = r#" + { + "gas_price": { + "eq": "0x12a05f200", + "lt": "0x12a05f200" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Action - let json = r#" - { - "gas_price": { - "action": "contract_creation" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + // Action + let json = r#" + { + "gas_price": { + "action": "contract_creation" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Unknown operator - let json = r#" - { - "gas_price": { - "abc": "0x0" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); - } + // Unknown operator + let json = r#" + { + "gas_price": { + "abc": "0x0" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + } - #[test] - fn valid_value_operators() { - // Eq - let json = r#" - { - "value": { - "eq": "0x0" - } - } - "#; - let default = FilterOptions::default(); - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - value: FilterOperator::Eq(U256::from(0)), - ..default.clone() - }); + #[test] + fn valid_value_operators() { + // Eq + let json = r#" + { + "value": { + "eq": "0x0" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + value: FilterOperator::Eq(U256::from(0)), + ..default.clone() + }); - // Gt - let json = r#" - { - "value": { - "gt": "0x0" - } - } - "#; - let default = FilterOptions::default(); - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - value: FilterOperator::GreaterThan(U256::from(0)), - ..default.clone() - }); + // Gt + let json = r#" + { + "value": { + "gt": "0x0" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + value: FilterOperator::GreaterThan(U256::from(0)), + ..default.clone() + }); - // Lt - let json = r#" - { - "value": { - "lt": "0x0" - } - } - "#; - let default = FilterOptions::default(); - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - value: FilterOperator::LessThan(U256::from(0)), - ..default - }); - } + // Lt + let json = r#" + { + "value": { + "lt": "0x0" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + value: FilterOperator::LessThan(U256::from(0)), + ..default + }); + } - #[test] - fn invalid_value_operators() { - // Multiple operators are invalid - let json = r#" - { - "value": { - "eq": "0x0", - "lt": "0x0" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + #[test] + fn invalid_value_operators() { + // Multiple operators are invalid + let json = r#" + { + "value": { + "eq": "0x0", + "lt": "0x0" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Action - let json = r#" - { - "value": { - "action": "contract_creation" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + // Action + let json = r#" + { + "value": { + "action": "contract_creation" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Unknown operator - let json = r#" - { - "value": { - "abc": "0x0" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); - } + // Unknown operator + let json = r#" + { + "value": { + "abc": "0x0" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + } - #[test] - fn valid_nonce_operators() { - // Eq - let json = r#" - { - "nonce": { - "eq": "0x577" - } - } - "#; - let default = FilterOptions::default(); - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - nonce: FilterOperator::Eq(U256::from(1399)), - ..default.clone() - }); + #[test] + fn valid_nonce_operators() { + // Eq + let json = r#" + { + "nonce": { + "eq": "0x577" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + nonce: FilterOperator::Eq(U256::from(1399)), + ..default.clone() + }); - // Gt - let json = r#" - { - "nonce": { - "gt": "0x577" - } - } - "#; - let default = FilterOptions::default(); - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - nonce: FilterOperator::GreaterThan(U256::from(1399)), - ..default.clone() - }); + // Gt + let json = r#" + { + "nonce": { + "gt": "0x577" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + nonce: FilterOperator::GreaterThan(U256::from(1399)), + ..default.clone() + }); - // Lt - let json = r#" - { - "nonce": { - "lt": "0x577" - } - } - "#; - let default = FilterOptions::default(); - let res = serde_json::from_str::(json).unwrap(); - assert_eq!(res, FilterOptions { - nonce: FilterOperator::LessThan(U256::from(1399)), - ..default - }); - } + // Lt + let json = r#" + { + "nonce": { + "lt": "0x577" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + nonce: FilterOperator::LessThan(U256::from(1399)), + ..default + }); + } - #[test] - fn invalid_nonce_operators() { - // Multiple operators are invalid - let json = r#" - { - "nonce": { - "eq": "0x577", - "lt": "0x577" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + #[test] + fn invalid_nonce_operators() { + // Multiple operators are invalid + let json = r#" + { + "nonce": { + "eq": "0x577", + "lt": "0x577" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Action - let json = r#" - { - "nonce": { - "action": "contract_creation" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); + // Action + let json = r#" + { + "nonce": { + "action": "contract_creation" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); - // Unknown operator - let json = r#" - { - "nonce": { - "abc": "0x0" - } - } - "#; - let res = serde_json::from_str::(json); - assert!(res.is_err()); - } + // Unknown operator + let json = r#" + { + "nonce": { + "abc": "0x0" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + } } diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index fef185e91..997ceb1bc 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -31,7 +31,7 @@ use ethcore_miner::work_notify::NotifyWork; use ethereum_types::{H256, U256, Address}; use futures::sync::mpsc; use io::IoChannel; -use miner::filter_options::{FilterOptions, FilterOperator}; +use miner::filter_options::FilterOptions; use miner::pool_client::{PoolClient, CachedNonceClient, NonceCache}; use miner::{self, MinerService}; use parking_lot::{Mutex, RwLock}; @@ -1095,7 +1095,8 @@ impl miner::MinerService for Miner { max_len: usize, filter: Option, ordering: miner::PendingOrdering, - ) -> Vec> where + ) -> Vec> + where C: ChainInfo + Nonce + Sync, { let chain_info = chain.chain_info(); @@ -1106,7 +1107,7 @@ impl miner::MinerService for Miner { // those transactions are valid and will just be ready to be included in next block. let nonce_cap = None; - self.transaction_queue.pending( + let mut pending = self.transaction_queue.pending( CachedNonceClient::new(chain, &self.nonce_cache), pool::PendingSettings { block_number: chain_info.best_block_number, @@ -1115,77 +1116,26 @@ impl miner::MinerService for Miner { max_len, ordering, }, - ) + ); + + pending.retain(|tx| { + filter.as_ref().map_or(true, |filter| { + filter.matches(tx.signed()) + }) + }); + + pending }; - use miner::filter_options::FilterOperator::*; let from_pending = || { self.map_existing_pending_block(|sealing| { - // This filter is used for gas, gas price, value and nonce. - // Sender and receiver have their custom matches, since those - // allow/disallow different operators. - fn match_common_filter(operator: &FilterOperator, tx_value: &U256) -> bool { - match operator { - Eq(value) => tx_value == value, - GreaterThan(value) => tx_value > value, - LessThan(value) => tx_value < value, - // Will always occur on `Any`, other operators - // get handled during deserialization - _ => true, - } - } - sealing.transactions .iter() .map(|signed| pool::VerifiedTransaction::from_pending_block_transaction(signed.clone())) .map(Arc::new) - // Filter by sender .filter(|tx| { filter.as_ref().map_or(true, |filter| { - let sender = tx.signed().sender(); - match filter.from { - Eq(value) => sender == value, - // Will always occur on `Any`, other operators - // get handled during deserialization - _ => true, - } - }) - }) - // Filter by receiver - .filter(|tx| { - filter.as_ref().map_or(true, |filter| { - let receiver = (*tx.signed()).receiver(); - match filter.to { - // Could apply to `Some(Address)` or `None` (for contract creation) - Eq(value) => receiver == value, - // Will always occur on `Any`, other operators - // get handled during deserialization - _ => true, - } - }) - }) - // Filter by gas - .filter(|tx| { - filter.as_ref().map_or(true, |filter| { - match_common_filter(&filter.gas, &(*tx.signed()).gas) - }) - }) - // Filter by gas price - .filter(|tx| { - filter.as_ref().map_or(true, |filter| { - match_common_filter(&filter.gas_price, &(*tx.signed()).gas_price) - }) - }) - // Filter by tx value - .filter(|tx| { - filter.as_ref().map_or(true, |filter| { - match_common_filter(&filter.value, &(*tx.signed()).value) - }) - }) - // Filter by nonce - .filter(|tx| { - filter.as_ref().map_or(true, |filter| { - match_common_filter(&filter.nonce, &(*tx.signed()).nonce) + filter.matches(tx.signed()) }) }) .take(max_len) @@ -1520,7 +1470,7 @@ mod tests { use client_traits::ChainInfo; use client::ImportSealedBlock; - use miner::{MinerService, PendingOrdering}; + use miner::{MinerService, PendingOrdering, filter_options::FilterOperator}; use test_helpers::{ generate_dummy_client, generate_dummy_client_with_spec, TestBlockChainClient, EachBlockWith }; @@ -1937,4 +1887,93 @@ mod tests { assert!(received_error_msg == expected_error_msg); } + + fn filter_tester(option: PendingSet) { + let client = TestBlockChainClient::default(); + let mut miner = miner(); + miner.options.pending_set = option; + let transaction = transaction(); + let best_block = 10; + let mut sender = transaction.sender(); + + miner.import_own_transaction(&client, PendingTransaction::new(transaction, None)).unwrap(); + + assert_eq!(miner.pending_transactions(best_block), None); + assert_eq!(miner.pending_receipts(best_block), None); + assert_eq!( + miner.ready_transactions_filtered(&client, 10, Some(FilterOptions::default()), PendingOrdering::Priority).len(), + 1 + ); + + // sender filter + { + // reverse address for false match + for byte in sender.as_bytes_mut() { + *byte = !*byte; + } + let mut filter = FilterOptions::default(); + filter.from = FilterOperator::Eq(sender); + assert_eq!( + miner.ready_transactions_filtered(&client, 10, Some(filter), PendingOrdering::Priority).len(), + 0 + ); + } + + // receiver filter + { + let mut filter = FilterOptions::default(); + filter.to = FilterOperator::Eq(Some(sender)); + assert_eq!( + miner.ready_transactions_filtered(&client, 10, Some(filter), PendingOrdering::Priority).len(), + 0 + ); + } + + // gas filter + { + let mut filter = FilterOptions::default(); + filter.gas = FilterOperator::LessThan(U256::from(100_000)); + assert_eq!( + miner.ready_transactions_filtered(&client, 10, Some(filter), PendingOrdering::Priority).len(), + 0 + ); + } + + // gas_price filter + { + let mut filter = FilterOptions::default(); + filter.gas_price = FilterOperator::GreaterThan(U256::from(10)); + assert_eq!( + miner.ready_transactions_filtered(&client, 10, Some(filter), PendingOrdering::Priority).len(), + 0 + ); + } + + // value filter + { + let mut filter = FilterOptions::default(); + filter.value = FilterOperator::Eq(U256::from(19)); + assert_eq!( + miner.ready_transactions_filtered(&client, 10, Some(filter), PendingOrdering::Priority).len(), + 0 + ); + } + + // nonce filter + { + let mut filter = FilterOptions::default(); + filter.nonce = FilterOperator::GreaterThan(U256::from(10)); + assert_eq!( + miner.ready_transactions_filtered(&client, 10, Some(filter), PendingOrdering::Priority).len(), + 0 + ); + } + } + + #[test] + fn transaction_filtering() { + filter_tester(PendingSet::AlwaysQueue); + filter_tester(PendingSet::AlwaysSealing); + filter_tester(PendingSet::SealingOrElseQueue); + } } diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index be7d83b57..27a4e9439 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -229,8 +229,23 @@ impl MinerService for TestMinerService { self.queued_transactions() } - fn ready_transactions_filtered(&self, _chain: &C, _max_len: usize, _filter: Option, _ordering: miner::PendingOrdering) -> Vec> { + fn ready_transactions_filtered( + &self, + _chain: &C, + max_len: usize, + filter: Option, + _ordering: miner::PendingOrdering + ) -> Vec> { self.queued_transactions() + .iter() + .cloned() + .filter(|tx| { + filter.as_ref().map_or(true, |filter| { + filter.matches(tx.signed()) + }) + }) + .take(max_len) + .collect() } fn pending_transaction_hashes(&self, _chain: &C) -> BTreeSet {