From 441b71cc8a2c05742c91f6435e082524afcf5911 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Mon, 18 Mar 2019 20:52:17 +0000 Subject: [PATCH 01/42] expand parameters for pending_transactions() --- rpc/src/v1/impls/light/parity.rs | 2 +- rpc/src/v1/impls/parity.rs | 2 +- rpc/src/v1/traits/parity.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rpc/src/v1/impls/light/parity.rs b/rpc/src/v1/impls/light/parity.rs index da7425e1f4c..1c649273418 100644 --- a/rpc/src/v1/impls/light/parity.rs +++ b/rpc/src/v1/impls/light/parity.rs @@ -207,7 +207,7 @@ where .map(Into::into) } - fn pending_transactions(&self, limit: Option) -> Result> { + fn pending_transactions(&self, limit: Option, tx_hash: Option, sender: Option, receiver: Option) -> Result> { let txq = self.light_dispatch.transaction_queue.read(); let chain_info = self.light_dispatch.client.chain_info(); Ok( diff --git a/rpc/src/v1/impls/parity.rs b/rpc/src/v1/impls/parity.rs index d985f33f625..173c5fd22bd 100644 --- a/rpc/src/v1/impls/parity.rs +++ b/rpc/src/v1/impls/parity.rs @@ -246,7 +246,7 @@ impl Parity for ParityClient where .map(Into::into) } - fn pending_transactions(&self, limit: Option) -> Result> { + fn pending_transactions(&self, limit: Option, tx_hash: Option, sender: Option, receiver: Option) -> Result> { let ready_transactions = self.miner.ready_transactions( &*self.client, limit.unwrap_or_else(usize::max_value), diff --git a/rpc/src/v1/traits/parity.rs b/rpc/src/v1/traits/parity.rs index e3821355eee..03ecec9e87a 100644 --- a/rpc/src/v1/traits/parity.rs +++ b/rpc/src/v1/traits/parity.rs @@ -125,7 +125,7 @@ pub trait Parity { /// Returns all pending transactions from transaction queue. #[rpc(name = "parity_pendingTransactions")] - fn pending_transactions(&self, Option) -> Result>; + fn pending_transactions(&self, Option, Option, Option, Option) -> Result>; /// Returns all transactions from transaction queue. /// From a2fcaf24a9e3914e3226093dfe0684fac0ea30fd Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Mon, 18 Mar 2019 21:36:50 +0000 Subject: [PATCH 02/42] move ready_transactions content into filtered method --- ethcore/src/miner/miner.rs | 15 ++++++++++++++- ethcore/src/miner/mod.rs | 5 ++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 7d6bcbe496d..c4b997ded3b 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -27,7 +27,7 @@ use ethcore_miner::local_accounts::LocalAccounts; use ethcore_miner::pool::{self, TransactionQueue, VerifiedTransaction, QueueStatus, PrioritizationStrategy}; #[cfg(feature = "work-notify")] use ethcore_miner::work_notify::NotifyWork; -use ethereum_types::{H256, U256, Address}; +use ethereum_types::{H160, H256, U256, Address}; use io::IoChannel; use miner::pool_client::{PoolClient, CachedNonceClient, NonceCache}; use miner; @@ -1018,6 +1018,15 @@ impl miner::MinerService for Miner { -> Vec> where C: ChainInfo + Nonce + Sync, + { + // No special filtering options applied (tx_hash, receiver or sender) + self.ready_transactions_filtered(chain, max_len, None, None, None, ordering) + } + + fn ready_transactions_filtered(&self, chain: &C, max_len: usize, tx_hash: Option, receiver: Option, sender: Option, ordering: miner::PendingOrdering) + -> Vec> + where + C: ChainInfo + Nonce + Sync, { let chain_info = chain.chain_info(); @@ -1045,6 +1054,10 @@ impl miner::MinerService for Miner { .iter() .map(|signed| pool::VerifiedTransaction::from_pending_block_transaction(signed.clone())) .map(Arc::new) + .filter(|tx| { + let _ = tx.pending(); + true + }) .take(max_len) .collect() }, chain_info.best_block_number) diff --git a/ethcore/src/miner/mod.rs b/ethcore/src/miner/mod.rs index fd7ab96513c..b6b82d8f044 100644 --- a/ethcore/src/miner/mod.rs +++ b/ethcore/src/miner/mod.rs @@ -34,7 +34,7 @@ use std::collections::{BTreeSet, BTreeMap}; use bytes::Bytes; use ethcore_miner::pool::{VerifiedTransaction, QueueStatus, local_transactions}; -use ethereum_types::{H256, U256, Address}; +use ethereum_types::{H160, H256, U256, Address}; use types::transaction::{self, UnverifiedTransaction, SignedTransaction, PendingTransaction}; use types::BlockNumber; use types::block::Block; @@ -184,6 +184,9 @@ pub trait MinerService : Send + Sync { fn ready_transactions(&self, chain: &C, max_len: usize, ordering: PendingOrdering) -> Vec> where C: ChainInfo + Nonce + Sync; + fn ready_transactions_filtered(&self, chain: &C, max_len: usize, tx_hash: Option, receiver: Option, sender: Option, ordering: PendingOrdering) -> Vec> + where C: ChainInfo + Nonce + Sync; + /// Get a list of all transactions in the pool (some of them might not be ready for inclusion yet). fn queued_transactions(&self) -> Vec>; From 636e466e26e02b605d59b2e1375c9ac18ffc3927 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Tue, 19 Mar 2019 19:12:58 +0000 Subject: [PATCH 03/42] apply filter based on tx_hash, sender or receiver --- ethcore/src/miner/miner.rs | 30 ++++++++++++++++++-- ethcore/types/src/transaction/transaction.rs | 13 +++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index c4b997ded3b..05f4316c617 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1019,7 +1019,7 @@ impl miner::MinerService for Miner { where C: ChainInfo + Nonce + Sync, { - // No special filtering options applied (tx_hash, receiver or sender) + // No special filtering options applied (neither tx_hash, receiver or sender) self.ready_transactions_filtered(chain, max_len, None, None, None, ordering) } @@ -1054,9 +1054,33 @@ impl miner::MinerService for Miner { .iter() .map(|signed| pool::VerifiedTransaction::from_pending_block_transaction(signed.clone())) .map(Arc::new) + // Filter by transaction hash .filter(|tx| { - let _ = tx.pending(); - true + if let Some(tx_hash) = tx_hash { + tx.signed().hash() == tx_hash + } else { + true // do not filter if None was passed + } + }) + // Filter by sender + .filter(|tx| { + if let Some(sender) = sender { + tx.signed().sender() == sender + } else { + true // do not filter if None was passed + } + }) + // Filter by receiver + .filter(|tx| { + if let Some(receiver) = receiver { + if let Some(tx_has_receiver) = tx.signed().receiver() { + tx_has_receiver == receiver + } else { + false // in case of Action::Create + } + } else { + true // do not filter if None was passed + } }) .take(max_len) .collect() diff --git a/ethcore/types/src/transaction/transaction.rs b/ethcore/types/src/transaction/transaction.rs index 248bc264629..3d55cc1d29e 100644 --- a/ethcore/types/src/transaction/transaction.rs +++ b/ethcore/types/src/transaction/transaction.rs @@ -460,6 +460,19 @@ impl SignedTransaction { self.sender } + /// Returns transaction receiver, if any + pub fn receiver(&self) -> Option
{ + match self.transaction.unsigned.action { + Action::Create => None, + Action::Call(receiver) => Some(receiver), + } + } + + /// Returns transaction hash + pub fn hash(&self) -> H256 { + self.transaction.hash + } + /// Returns a public key of the sender. pub fn public_key(&self) -> Option { self.public From 325eb6f1eb407eafa44c4d6641db257afee287cb Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Tue, 19 Mar 2019 19:22:10 +0000 Subject: [PATCH 04/42] call filtered transactions from RPC interface --- rpc/src/v1/impls/parity.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rpc/src/v1/impls/parity.rs b/rpc/src/v1/impls/parity.rs index 173c5fd22bd..323b6c307bc 100644 --- a/rpc/src/v1/impls/parity.rs +++ b/rpc/src/v1/impls/parity.rs @@ -247,9 +247,12 @@ impl Parity for ParityClient where } fn pending_transactions(&self, limit: Option, tx_hash: Option, sender: Option, receiver: Option) -> Result> { - let ready_transactions = self.miner.ready_transactions( + let ready_transactions = self.miner.ready_transactions_filtered( &*self.client, limit.unwrap_or_else(usize::max_value), + tx_hash, + sender, + receiver, miner::PendingOrdering::Priority, ); From 2dc0f32e2fa5dfe77e44354debd0e77ab64644e8 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Thu, 21 Mar 2019 21:20:16 +0000 Subject: [PATCH 05/42] attempt at testing... --- ethcore/src/miner/miner.rs | 48 ++++++++++++++++++++++- rpc/src/v1/tests/helpers/miner_service.rs | 6 ++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 05f4316c617..4a7d2343b8f 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1363,7 +1363,7 @@ mod tests { use super::*; use accounts::AccountProvider; - use ethkey::{Generator, Random}; + use ethkey::{Generator, Random, Secret}; use hash::keccak; use rustc_hex::FromHex; use types::BlockNumber; @@ -1455,6 +1455,35 @@ mod tests { }.sign(keypair.secret(), Some(chain_id)) } + fn transactions() -> Vec { + // Create a single, reproducible keypair (sender) + let keypair1 = Secret::from(H256::from(1)); + let keypair2 = Secret::from(H256::from(2)); + + // Create two different, reproducible addresses (receivers) + let address1 = H160::from(1); + let address2 = H160::from(2); + + vec![ + Transaction { + action: Action::Call(address1), + value: U256::zero(), + data: "3331600055".from_hex().unwrap(), + gas: U256::from(100_000), + gas_price: U256::zero(), + nonce: U256::zero(), + }.sign(&keypair1, Some(TEST_CHAIN_ID)), + Transaction { + action: Action::Call(address2), + value: U256::zero(), + data: "3331600055".from_hex().unwrap(), + gas: U256::from(100_000), + gas_price: U256::zero(), + nonce: U256::zero(), + }.sign(&keypair2, Some(TEST_CHAIN_ID)), + ] + } + #[test] fn should_make_pending_block_when_importing_own_transaction() { // given @@ -1773,4 +1802,21 @@ mod tests { assert!(received_error_msg == expected_error_msg); } + + #[test] + fn filter_pending_transactions_by_tx_hash() { + // given + let client = TestBlockChainClient::default(); + let miner = miner(); + let pending_transactions = transactions(); + // when + for transaction in pending_transactions { + let res = miner.import_own_transaction(&client, PendingTransaction::new(transaction, None)); + assert_eq!(res.unwrap(), ()); + } + + // then + let res = miner.ready_transactions_filtered(&client, 10, None, None, None, PendingOrdering::Unordered); + // ... ready_transactions() always seems to return only two transactions? + } } diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index 77618c85328..4960c0dbcc8 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -25,7 +25,7 @@ use ethcore::client::{Nonce, PrepareOpenBlock, StateClient, EngineInfo}; use ethcore::engines::{EthEngine, signer::EngineSigner}; use ethcore::error::Error; use ethcore::miner::{self, MinerService, AuthoringParams}; -use ethereum_types::{H256, U256, Address}; +use ethereum_types::{H160, H256, U256, Address}; use miner::pool::local_transactions::Status as LocalTransactionStatus; use miner::pool::{verifier, VerifiedTransaction, QueueStatus}; use parking_lot::{RwLock, Mutex}; @@ -222,6 +222,10 @@ impl MinerService for TestMinerService { self.queued_transactions() } + fn ready_transactions_filtered(&self, _chain: &C, _max_len: usize, _tx_hash: Option, _sender: Option, _receiver: Option, _ordering: miner::PendingOrdering) -> Vec> { + self.queued_transactions() + } + fn pending_transaction_hashes(&self, _chain: &C) -> BTreeSet { self.queued_transactions().into_iter().map(|tx| tx.signed().hash()).collect() } From 8d840fbbf53ce0a84a7be27add9527a6082738ff Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Thu, 21 Mar 2019 22:29:27 +0000 Subject: [PATCH 06/42] replace parameters with _ on light client --- rpc/src/v1/impls/light/parity.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rpc/src/v1/impls/light/parity.rs b/rpc/src/v1/impls/light/parity.rs index 1c649273418..e1436a429a8 100644 --- a/rpc/src/v1/impls/light/parity.rs +++ b/rpc/src/v1/impls/light/parity.rs @@ -207,7 +207,7 @@ where .map(Into::into) } - fn pending_transactions(&self, limit: Option, tx_hash: Option, sender: Option, receiver: Option) -> Result> { + fn pending_transactions(&self, limit: Option, _: Option, _: Option, _: Option) -> Result> { let txq = self.light_dispatch.transaction_queue.read(); let chain_info = self.light_dispatch.client.chain_info(); Ok( From 2862dc8b5e38e041c72a30351a7159258c6c87b5 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Thu, 21 Mar 2019 22:54:52 +0000 Subject: [PATCH 07/42] addes some comments --- ethcore/src/miner/miner.rs | 4 +++- ethcore/src/miner/mod.rs | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 4a7d2343b8f..669049f6296 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1455,8 +1455,9 @@ mod tests { }.sign(keypair.secret(), Some(chain_id)) } + // TODO... fn transactions() -> Vec { - // Create a single, reproducible keypair (sender) + // Create two different, reproducible keypairs (senders) let keypair1 = Secret::from(H256::from(1)); let keypair2 = Secret::from(H256::from(2)); @@ -1818,5 +1819,6 @@ mod tests { // then let res = miner.ready_transactions_filtered(&client, 10, None, None, None, PendingOrdering::Unordered); // ... ready_transactions() always seems to return only two transactions? + // TODO... } } diff --git a/ethcore/src/miner/mod.rs b/ethcore/src/miner/mod.rs index b6b82d8f044..1e05b08e281 100644 --- a/ethcore/src/miner/mod.rs +++ b/ethcore/src/miner/mod.rs @@ -184,6 +184,11 @@ pub trait MinerService : Send + Sync { fn ready_transactions(&self, chain: &C, max_len: usize, ordering: PendingOrdering) -> Vec> where C: ChainInfo + Nonce + Sync; + /// Get a list of all ready transactions either ordered by priority or unordered (cheaper), optionally filtered by hash, sender or receiver. + /// + /// Depending on the settings may look in transaction pool or only in pending block. + /// If you don't need a full set of transactions, you can add `max_len` and create only a limited set of + /// transactions. fn ready_transactions_filtered(&self, chain: &C, max_len: usize, tx_hash: Option, receiver: Option, sender: Option, ordering: PendingOrdering) -> Vec> where C: ChainInfo + Nonce + Sync; From 9578d497ad24873277c7b29399d0a4eb05017ffe Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Tue, 2 Apr 2019 18:14:29 +0000 Subject: [PATCH 08/42] removed uncompleted tests in miner.rs --- ethcore/src/miner/miner.rs | 50 +------------------------------------- 1 file changed, 1 insertion(+), 49 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 669049f6296..05f4316c617 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1363,7 +1363,7 @@ mod tests { use super::*; use accounts::AccountProvider; - use ethkey::{Generator, Random, Secret}; + use ethkey::{Generator, Random}; use hash::keccak; use rustc_hex::FromHex; use types::BlockNumber; @@ -1455,36 +1455,6 @@ mod tests { }.sign(keypair.secret(), Some(chain_id)) } - // TODO... - fn transactions() -> Vec { - // Create two different, reproducible keypairs (senders) - let keypair1 = Secret::from(H256::from(1)); - let keypair2 = Secret::from(H256::from(2)); - - // Create two different, reproducible addresses (receivers) - let address1 = H160::from(1); - let address2 = H160::from(2); - - vec![ - Transaction { - action: Action::Call(address1), - value: U256::zero(), - data: "3331600055".from_hex().unwrap(), - gas: U256::from(100_000), - gas_price: U256::zero(), - nonce: U256::zero(), - }.sign(&keypair1, Some(TEST_CHAIN_ID)), - Transaction { - action: Action::Call(address2), - value: U256::zero(), - data: "3331600055".from_hex().unwrap(), - gas: U256::from(100_000), - gas_price: U256::zero(), - nonce: U256::zero(), - }.sign(&keypair2, Some(TEST_CHAIN_ID)), - ] - } - #[test] fn should_make_pending_block_when_importing_own_transaction() { // given @@ -1803,22 +1773,4 @@ mod tests { assert!(received_error_msg == expected_error_msg); } - - #[test] - fn filter_pending_transactions_by_tx_hash() { - // given - let client = TestBlockChainClient::default(); - let miner = miner(); - let pending_transactions = transactions(); - // when - for transaction in pending_transactions { - let res = miner.import_own_transaction(&client, PendingTransaction::new(transaction, None)); - assert_eq!(res.unwrap(), ()); - } - - // then - let res = miner.ready_transactions_filtered(&client, 10, None, None, None, PendingOrdering::Unordered); - // ... ready_transactions() always seems to return only two transactions? - // TODO... - } } From 6f1e437b026b6556a41e50be1f7dbaf0a10de550 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Fri, 5 Apr 2019 18:45:46 +0000 Subject: [PATCH 09/42] attempt at testing, needs more work... --- rpc/src/v1/tests/mocked/parity.rs | 94 +++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/rpc/src/v1/tests/mocked/parity.rs b/rpc/src/v1/tests/mocked/parity.rs index 6608589f557..567e23c3c80 100644 --- a/rpc/src/v1/tests/mocked/parity.rs +++ b/rpc/src/v1/tests/mocked/parity.rs @@ -322,6 +322,100 @@ fn rpc_parity_pending_transactions() { assert_eq!(io.handle_request_sync(request), Some(response.to_owned())); } +#[test] +fn rpc_parity_pending_transactions_filtered() { + let deps = Dependencies::new(); + let io = deps.default_client(); + let tx1 = ::types::transaction::Transaction { + value: 5.into(), + gas: 3.into(), + gas_price: 2.into(), + action: ::types::transaction::Action::Call(Address::from(1_000)), // same receiver as tx2 + data: vec![1, 2, 3], + nonce: 0.into(), + }.fake_sign(10.into()); // same sender as tx3 + let content_tx1 = r#" + { + "blockHash": null, + "blockNumber": null, + "chainId": null, + "condition": null, + "creates": null, + "from": "0x000000000000000000000000000000000000000a", + "gas": "0x3", + "gasPrice": "0x2", + "hash": "0xb8d44001a15402b927e6f1f397f0d61d67e560f0376ef01c13d04b0f0a87adb8", + "input": "0x010203", + "nonce": "0x0", + "publicKey": null, + "r": "0x1", + "raw": "0xe08002039400000000000000000000000000000000000003e80583010203800101", + "s": "0x1", + "standardV": "0x4", + "to": "0x00000000000000000000000000000000000003e8", + "transactionIndex": null, + "v": "0x0", + "value": "0x5" + } + "#; + + let tx2 = ::types::transaction::Transaction { + value: 5.into(), + gas: 3.into(), + gas_price: 2.into(), + action: ::types::transaction::Action::Call(Address::from(1_000)), // same receiver as tx1 + data: vec![1, 2, 3], + nonce: 0.into(), + }.fake_sign(20.into()); + let content_tx2 = r#" + { + "blockHash": null, + "blockNumber": null, + "chainId": null, + "condition": null, + "creates": null, + "from": "0x0000000000000000000000000000000000000014", + "gas": "0x3", + "gasPrice": "0x2", + "hash": "0xb8d44001a15402b927e6f1f397f0d61d67e560f0376ef01c13d04b0f0a87adb8", + "input": "0x010203", + "nonce": "0x0", + "publicKey": null, + "r": "0x1", + "raw": "0xe08002039400000000000000000000000000000000000003e80583010203800101", + "s": "0x1", + "standardV": "0x4", + "to": "0x00000000000000000000000000000000000003e8", + "transactionIndex": null, + "v": "0x0", + "value": "0x5" + } + "#; + + let tx3 = ::types::transaction::Transaction { + value: 5.into(), + gas: 3.into(), + gas_price: 2.into(), + action: ::types::transaction::Action::Call(Address::from(2_000)), + data: vec![1, 2, 3], + nonce: 0.into(), + }.fake_sign(10.into()); // same sender as tx1 + let content_tx3 = r#" + + "#; + + //deps.miner.pending_transactions.lock().insert(H256::from(100), tx1); + deps.miner.pending_transactions.lock().insert(H256::from(0), tx2); + //deps.miner.pending_transactions.lock().insert(H256::from(300), tx3); + + let request = r#"{"jsonrpc": "2.0", "method": "parity_pendingTransactions", "params":[], "id": 1}"#; + + println!("RES: {:?}", io.handle_request_sync(request)); + println!("\n\n\n"); + + //assert_eq!(io.handle_request_sync(request), Some(response_all.to_owned())); +} + #[test] fn rpc_parity_encrypt() { let deps = Dependencies::new(); From 56decec7d5f3ca5a557863382cf454a3d55bf83b Mon Sep 17 00:00:00 2001 From: Fabio Lama <42901763+lamafab@users.noreply.github.com> Date: Mon, 10 Jun 2019 04:51:12 +0200 Subject: [PATCH 10/42] Formatting for ready_transactions_filtered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Tomasz Drwięga --- ethcore/src/miner/miner.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 67e644a40d8..6600ed67f07 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1055,7 +1055,17 @@ impl miner::MinerService for Miner { self.ready_transactions_filtered(chain, max_len, None, None, None, ordering) } - fn ready_transactions_filtered(&self, chain: &C, max_len: usize, tx_hash: Option, receiver: Option, sender: Option, ordering: miner::PendingOrdering) + fn ready_transactions_filtered( + &self, + chain: &C, + max_len: usize, + tx_hash: Option, + receiver: Option, + sender: Option, + ordering: miner::PendingOrdering, + ) -> Vec> where + C: ChainInfo + Nonce + Sync, + { -> Vec> where C: ChainInfo + Nonce + Sync, From 83cbc3ebc5b02236e689ea029e6ff21466001738 Mon Sep 17 00:00:00 2001 From: Fabio Lama <42901763+lamafab@users.noreply.github.com> Date: Mon, 10 Jun 2019 04:52:06 +0200 Subject: [PATCH 11/42] Use map_or instead of if-let MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Tomasz Drwięga --- ethcore/src/miner/miner.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 6600ed67f07..521ee192e25 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1098,7 +1098,7 @@ impl miner::MinerService for Miner { .map(Arc::new) // Filter by transaction hash .filter(|tx| { - if let Some(tx_hash) = tx_hash { + tx_hash.map_or(true, |tx_hash| tx.signed().hash() == tx_hash) tx.signed().hash() == tx_hash } else { true // do not filter if None was passed From 27161c8df96faf92f4b17348afc361867a8d61b4 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Mon, 10 Jun 2019 04:55:45 +0200 Subject: [PATCH 12/42] additional map_or replacement --- ethcore/src/miner/miner.rs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 521ee192e25..bef72e44f6a 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1065,10 +1065,6 @@ impl miner::MinerService for Miner { ordering: miner::PendingOrdering, ) -> Vec> where C: ChainInfo + Nonce + Sync, - { - -> Vec> - where - C: ChainInfo + Nonce + Sync, { let chain_info = chain.chain_info(); @@ -1099,18 +1095,10 @@ impl miner::MinerService for Miner { // Filter by transaction hash .filter(|tx| { tx_hash.map_or(true, |tx_hash| tx.signed().hash() == tx_hash) - tx.signed().hash() == tx_hash - } else { - true // do not filter if None was passed - } }) // Filter by sender .filter(|tx| { - if let Some(sender) = sender { - tx.signed().sender() == sender - } else { - true // do not filter if None was passed - } + sender.map_or(true, |sender| tx.signed().sender() == sender) }) // Filter by receiver .filter(|tx| { From c06b50c83de8a5e517413070ccc5eded74cd7904 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Mon, 10 Jun 2019 06:18:28 +0200 Subject: [PATCH 13/42] change receiver type to Option> --- ethcore/src/miner/miner.rs | 16 ++++------------ ethcore/src/miner/mod.rs | 4 ++-- rpc/src/v1/impls/light/parity.rs | 2 +- rpc/src/v1/impls/parity.rs | 2 +- rpc/src/v1/traits/parity.rs | 2 +- 5 files changed, 9 insertions(+), 17 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index bef72e44f6a..c41b32e657f 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -28,7 +28,7 @@ use ethcore_miner::pool::{self, TransactionQueue, VerifiedTransaction, QueueStat use ethcore_miner::service_transaction_checker::ServiceTransactionChecker; #[cfg(feature = "work-notify")] use ethcore_miner::work_notify::NotifyWork; -use ethereum_types::{H160, H256, U256, Address}; +use ethereum_types::{H256, U256, Address}; use futures::sync::mpsc; use io::IoChannel; use miner::pool_client::{PoolClient, CachedNonceClient, NonceCache}; @@ -1060,8 +1060,8 @@ impl miner::MinerService for Miner { chain: &C, max_len: usize, tx_hash: Option, - receiver: Option, - sender: Option, + sender: Option
, + receiver: Option>, ordering: miner::PendingOrdering, ) -> Vec> where C: ChainInfo + Nonce + Sync, @@ -1102,15 +1102,7 @@ impl miner::MinerService for Miner { }) // Filter by receiver .filter(|tx| { - if let Some(receiver) = receiver { - if let Some(tx_has_receiver) = tx.signed().receiver() { - tx_has_receiver == receiver - } else { - false // in case of Action::Create - } - } else { - true // do not filter if None was passed - } + receiver.map_or(true, |receiver| tx.signed().receiver() == receiver) }) .take(max_len) .collect() diff --git a/ethcore/src/miner/mod.rs b/ethcore/src/miner/mod.rs index 1e05b08e281..890a7a98ba6 100644 --- a/ethcore/src/miner/mod.rs +++ b/ethcore/src/miner/mod.rs @@ -34,7 +34,7 @@ use std::collections::{BTreeSet, BTreeMap}; use bytes::Bytes; use ethcore_miner::pool::{VerifiedTransaction, QueueStatus, local_transactions}; -use ethereum_types::{H160, H256, U256, Address}; +use ethereum_types::{H256, U256, Address}; use types::transaction::{self, UnverifiedTransaction, SignedTransaction, PendingTransaction}; use types::BlockNumber; use types::block::Block; @@ -189,7 +189,7 @@ pub trait MinerService : Send + Sync { /// Depending on the settings may look in transaction pool or only in pending block. /// If you don't need a full set of transactions, you can add `max_len` and create only a limited set of /// transactions. - fn ready_transactions_filtered(&self, chain: &C, max_len: usize, tx_hash: Option, receiver: Option, sender: Option, ordering: PendingOrdering) -> Vec> + fn ready_transactions_filtered(&self, chain: &C, max_len: usize, tx_hash: Option, sender: Option
, receiver: Option>, ordering: PendingOrdering) -> Vec> where C: ChainInfo + Nonce + Sync; /// Get a list of all transactions in the pool (some of them might not be ready for inclusion yet). diff --git a/rpc/src/v1/impls/light/parity.rs b/rpc/src/v1/impls/light/parity.rs index d76e35e4a8a..bc80da69b4f 100644 --- a/rpc/src/v1/impls/light/parity.rs +++ b/rpc/src/v1/impls/light/parity.rs @@ -217,7 +217,7 @@ where .map(Into::into) } - fn pending_transactions(&self, limit: Option, _: Option, _: Option, _: Option) -> Result> { + fn pending_transactions(&self, limit: Option, _: Option, _: Option, _: Option>) -> Result> { let txq = self.light_dispatch.transaction_queue.read(); let chain_info = self.light_dispatch.client.chain_info(); Ok( diff --git a/rpc/src/v1/impls/parity.rs b/rpc/src/v1/impls/parity.rs index 75556eb316b..ac0c1e5e704 100644 --- a/rpc/src/v1/impls/parity.rs +++ b/rpc/src/v1/impls/parity.rs @@ -245,7 +245,7 @@ impl Parity for ParityClient where .map(Into::into) } - fn pending_transactions(&self, limit: Option, tx_hash: Option, sender: Option, receiver: Option) -> Result> { + fn pending_transactions(&self, limit: Option, tx_hash: Option, sender: Option
, receiver: Option>) -> Result> { let ready_transactions = self.miner.ready_transactions_filtered( &*self.client, limit.unwrap_or_else(usize::max_value), diff --git a/rpc/src/v1/traits/parity.rs b/rpc/src/v1/traits/parity.rs index 10c847b5b68..e249205b90d 100644 --- a/rpc/src/v1/traits/parity.rs +++ b/rpc/src/v1/traits/parity.rs @@ -125,7 +125,7 @@ pub trait Parity { /// Returns all pending transactions from transaction queue. #[rpc(name = "parity_pendingTransactions")] - fn pending_transactions(&self, Option, Option, Option, Option) -> Result>; + fn pending_transactions(&self, Option, Option, Option, Option>) -> Result>; /// Returns all transactions from transaction queue. /// From 3f92d7b834b027c1066e44cd705084b88da8ece3 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Mon, 10 Jun 2019 06:51:18 +0200 Subject: [PATCH 14/42] remove faulty MiningService tests, test RPC later --- rpc/src/v1/impls/light/parity.rs | 2 +- rpc/src/v1/tests/helpers/miner_service.rs | 4 +- rpc/src/v1/tests/mocked/parity.rs | 94 ----------------------- 3 files changed, 3 insertions(+), 97 deletions(-) diff --git a/rpc/src/v1/impls/light/parity.rs b/rpc/src/v1/impls/light/parity.rs index bc80da69b4f..63e4acb4b38 100644 --- a/rpc/src/v1/impls/light/parity.rs +++ b/rpc/src/v1/impls/light/parity.rs @@ -217,7 +217,7 @@ where .map(Into::into) } - fn pending_transactions(&self, limit: Option, _: Option, _: Option, _: Option>) -> Result> { + fn pending_transactions(&self, limit: Option, _tx_hash: Option, _sender: Option, _receiver: Option>) -> Result> { let txq = self.light_dispatch.transaction_queue.read(); let chain_info = self.light_dispatch.client.chain_info(); Ok( diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index 0049caa0e21..47dcf79f52e 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -25,7 +25,7 @@ use ethcore::client::{Nonce, PrepareOpenBlock, StateClient, EngineInfo}; use ethcore::engines::{EthEngine, signer::EngineSigner}; use ethcore::error::Error; use ethcore::miner::{self, MinerService, AuthoringParams}; -use ethereum_types::{H160, H256, U256, Address}; +use ethereum_types::{H256, U256, Address}; use miner::pool::local_transactions::Status as LocalTransactionStatus; use miner::pool::{verifier, VerifiedTransaction, QueueStatus}; use parking_lot::{RwLock, Mutex}; @@ -222,7 +222,7 @@ impl MinerService for TestMinerService { self.queued_transactions() } - fn ready_transactions_filtered(&self, _chain: &C, _max_len: usize, _tx_hash: Option, _sender: Option, _receiver: Option, _ordering: miner::PendingOrdering) -> Vec> { + fn ready_transactions_filtered(&self, _chain: &C, _max_len: usize, _tx_hash: Option, _sender: Option
, _receiver: Option>, _ordering: miner::PendingOrdering) -> Vec> { self.queued_transactions() } diff --git a/rpc/src/v1/tests/mocked/parity.rs b/rpc/src/v1/tests/mocked/parity.rs index c08b952430c..913e181e400 100644 --- a/rpc/src/v1/tests/mocked/parity.rs +++ b/rpc/src/v1/tests/mocked/parity.rs @@ -322,100 +322,6 @@ fn rpc_parity_pending_transactions() { assert_eq!(io.handle_request_sync(request), Some(response.to_owned())); } -#[test] -fn rpc_parity_pending_transactions_filtered() { - let deps = Dependencies::new(); - let io = deps.default_client(); - let tx1 = ::types::transaction::Transaction { - value: 5.into(), - gas: 3.into(), - gas_price: 2.into(), - action: ::types::transaction::Action::Call(Address::from(1_000)), // same receiver as tx2 - data: vec![1, 2, 3], - nonce: 0.into(), - }.fake_sign(10.into()); // same sender as tx3 - let content_tx1 = r#" - { - "blockHash": null, - "blockNumber": null, - "chainId": null, - "condition": null, - "creates": null, - "from": "0x000000000000000000000000000000000000000a", - "gas": "0x3", - "gasPrice": "0x2", - "hash": "0xb8d44001a15402b927e6f1f397f0d61d67e560f0376ef01c13d04b0f0a87adb8", - "input": "0x010203", - "nonce": "0x0", - "publicKey": null, - "r": "0x1", - "raw": "0xe08002039400000000000000000000000000000000000003e80583010203800101", - "s": "0x1", - "standardV": "0x4", - "to": "0x00000000000000000000000000000000000003e8", - "transactionIndex": null, - "v": "0x0", - "value": "0x5" - } - "#; - - let tx2 = ::types::transaction::Transaction { - value: 5.into(), - gas: 3.into(), - gas_price: 2.into(), - action: ::types::transaction::Action::Call(Address::from(1_000)), // same receiver as tx1 - data: vec![1, 2, 3], - nonce: 0.into(), - }.fake_sign(20.into()); - let content_tx2 = r#" - { - "blockHash": null, - "blockNumber": null, - "chainId": null, - "condition": null, - "creates": null, - "from": "0x0000000000000000000000000000000000000014", - "gas": "0x3", - "gasPrice": "0x2", - "hash": "0xb8d44001a15402b927e6f1f397f0d61d67e560f0376ef01c13d04b0f0a87adb8", - "input": "0x010203", - "nonce": "0x0", - "publicKey": null, - "r": "0x1", - "raw": "0xe08002039400000000000000000000000000000000000003e80583010203800101", - "s": "0x1", - "standardV": "0x4", - "to": "0x00000000000000000000000000000000000003e8", - "transactionIndex": null, - "v": "0x0", - "value": "0x5" - } - "#; - - let tx3 = ::types::transaction::Transaction { - value: 5.into(), - gas: 3.into(), - gas_price: 2.into(), - action: ::types::transaction::Action::Call(Address::from(2_000)), - data: vec![1, 2, 3], - nonce: 0.into(), - }.fake_sign(10.into()); // same sender as tx1 - let content_tx3 = r#" - - "#; - - //deps.miner.pending_transactions.lock().insert(H256::from(100), tx1); - deps.miner.pending_transactions.lock().insert(H256::from(0), tx2); - //deps.miner.pending_transactions.lock().insert(H256::from(300), tx3); - - let request = r#"{"jsonrpc": "2.0", "method": "parity_pendingTransactions", "params":[], "id": 1}"#; - - println!("RES: {:?}", io.handle_request_sync(request)); - println!("\n\n\n"); - - //assert_eq!(io.handle_request_sync(request), Some(response_all.to_owned())); -} - #[test] fn rpc_parity_encrypt() { let deps = Dependencies::new(); From 2254aeeb9e53a90dbb7f31739db94038910a5940 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Mon, 10 Jun 2019 06:59:21 +0200 Subject: [PATCH 15/42] remove tx hash from pending transaction filtering --- ethcore/src/miner/miner.rs | 7 +------ ethcore/src/miner/mod.rs | 2 +- rpc/src/v1/impls/light/parity.rs | 2 +- rpc/src/v1/impls/parity.rs | 3 +-- rpc/src/v1/tests/helpers/miner_service.rs | 2 +- rpc/src/v1/traits/parity.rs | 2 +- 6 files changed, 6 insertions(+), 12 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index c41b32e657f..7bb80047f18 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1052,14 +1052,13 @@ impl miner::MinerService for Miner { C: ChainInfo + Nonce + Sync, { // No special filtering options applied (neither tx_hash, receiver or sender) - self.ready_transactions_filtered(chain, max_len, None, None, None, ordering) + self.ready_transactions_filtered(chain, max_len, None, None, ordering) } fn ready_transactions_filtered( &self, chain: &C, max_len: usize, - tx_hash: Option, sender: Option
, receiver: Option>, ordering: miner::PendingOrdering, @@ -1092,10 +1091,6 @@ impl miner::MinerService for Miner { .iter() .map(|signed| pool::VerifiedTransaction::from_pending_block_transaction(signed.clone())) .map(Arc::new) - // Filter by transaction hash - .filter(|tx| { - tx_hash.map_or(true, |tx_hash| tx.signed().hash() == tx_hash) - }) // Filter by sender .filter(|tx| { sender.map_or(true, |sender| tx.signed().sender() == sender) diff --git a/ethcore/src/miner/mod.rs b/ethcore/src/miner/mod.rs index 890a7a98ba6..395f07cdf10 100644 --- a/ethcore/src/miner/mod.rs +++ b/ethcore/src/miner/mod.rs @@ -189,7 +189,7 @@ pub trait MinerService : Send + Sync { /// Depending on the settings may look in transaction pool or only in pending block. /// If you don't need a full set of transactions, you can add `max_len` and create only a limited set of /// transactions. - fn ready_transactions_filtered(&self, chain: &C, max_len: usize, tx_hash: Option, sender: Option
, receiver: Option>, ordering: PendingOrdering) -> Vec> + fn ready_transactions_filtered(&self, chain: &C, max_len: usize, sender: Option
, receiver: Option>, ordering: PendingOrdering) -> Vec> where C: ChainInfo + Nonce + Sync; /// Get a list of all transactions in the pool (some of them might not be ready for inclusion yet). diff --git a/rpc/src/v1/impls/light/parity.rs b/rpc/src/v1/impls/light/parity.rs index 63e4acb4b38..2e390796b58 100644 --- a/rpc/src/v1/impls/light/parity.rs +++ b/rpc/src/v1/impls/light/parity.rs @@ -217,7 +217,7 @@ where .map(Into::into) } - fn pending_transactions(&self, limit: Option, _tx_hash: Option, _sender: Option, _receiver: Option>) -> Result> { + fn pending_transactions(&self, limit: Option, _sender: Option, _receiver: Option>) -> Result> { let txq = self.light_dispatch.transaction_queue.read(); let chain_info = self.light_dispatch.client.chain_info(); Ok( diff --git a/rpc/src/v1/impls/parity.rs b/rpc/src/v1/impls/parity.rs index ac0c1e5e704..28f8ee3224a 100644 --- a/rpc/src/v1/impls/parity.rs +++ b/rpc/src/v1/impls/parity.rs @@ -245,11 +245,10 @@ impl Parity for ParityClient where .map(Into::into) } - fn pending_transactions(&self, limit: Option, tx_hash: Option, sender: Option
, receiver: Option>) -> Result> { + fn pending_transactions(&self, limit: Option, sender: Option
, receiver: Option>) -> Result> { let ready_transactions = self.miner.ready_transactions_filtered( &*self.client, limit.unwrap_or_else(usize::max_value), - tx_hash, sender, receiver, miner::PendingOrdering::Priority, diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index 47dcf79f52e..68b7c33bc42 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -222,7 +222,7 @@ impl MinerService for TestMinerService { self.queued_transactions() } - fn ready_transactions_filtered(&self, _chain: &C, _max_len: usize, _tx_hash: Option, _sender: Option
, _receiver: Option>, _ordering: miner::PendingOrdering) -> Vec> { + fn ready_transactions_filtered(&self, _chain: &C, _max_len: usize, _sender: Option
, _receiver: Option>, _ordering: miner::PendingOrdering) -> Vec> { self.queued_transactions() } diff --git a/rpc/src/v1/traits/parity.rs b/rpc/src/v1/traits/parity.rs index e249205b90d..bc37da789cb 100644 --- a/rpc/src/v1/traits/parity.rs +++ b/rpc/src/v1/traits/parity.rs @@ -125,7 +125,7 @@ pub trait Parity { /// Returns all pending transactions from transaction queue. #[rpc(name = "parity_pendingTransactions")] - fn pending_transactions(&self, Option, Option, Option, Option>) -> Result>; + fn pending_transactions(&self, Option, Option, Option>) -> Result>; /// Returns all transactions from transaction queue. /// From bee31cb6574a90cbc7fba5af3d5e8f508da8eb64 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Tue, 11 Jun 2019 19:17:30 +0000 Subject: [PATCH 16/42] as_unsigned() method for SignedTransaction --- ethcore/types/src/transaction/transaction.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ethcore/types/src/transaction/transaction.rs b/ethcore/types/src/transaction/transaction.rs index c28f674048b..d031d95ab4e 100644 --- a/ethcore/types/src/transaction/transaction.rs +++ b/ethcore/types/src/transaction/transaction.rs @@ -470,6 +470,11 @@ impl SignedTransaction { } } + // Reference to the plain text transaction + pub fn as_unsigned(&self) -> &Transaction { + self.transaction.as_unsigned() + } + /// Returns transaction hash pub fn hash(&self) -> H256 { self.transaction.hash From c5b745633c39964d3399242e44bbe9c90613798c Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Thu, 20 Jun 2019 20:06:41 +0000 Subject: [PATCH 17/42] implement Deserialize for FilterOptions --- rpc/src/v1/helpers/filter_options.rs | 182 +++++++++++++++++++++++++++ rpc/src/v1/helpers/mod.rs | 2 + 2 files changed, 184 insertions(+) create mode 100644 rpc/src/v1/helpers/filter_options.rs diff --git a/rpc/src/v1/helpers/filter_options.rs b/rpc/src/v1/helpers/filter_options.rs new file mode 100644 index 00000000000..cae538be33b --- /dev/null +++ b/rpc/src/v1/helpers/filter_options.rs @@ -0,0 +1,182 @@ +use ethereum_types::{Address, U256}; +use serde::de::{Deserialize, Deserializer, Error, MapAccess, Visitor}; +use std::fmt; +use std::marker::PhantomData; + +#[derive(Debug, Clone)] +pub struct FilterOptions { + sender: FilterOperator
, + receiver: FilterOperator>, + gas: FilterOperator, + gas_price: FilterOperator, + value: FilterOperator, + nonce: FilterOperator, +} + +impl Default for FilterOptions { + fn default() -> Self { + FilterOptions { + sender: FilterOperator::Any, + receiver: FilterOperator::Any, + gas: FilterOperator::Any, + gas_price: FilterOperator::Any, + value: FilterOperator::Any, + nonce: FilterOperator::Any, + } + } +} + +#[derive(Debug, Clone)] +pub enum FilterOperator { + Any, + Eq(T), + GreaterThan(T), + LessThan(T), + ContractCreation, // only used for `receiver` +} + +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 expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + // "This Visitor expects to receive ..." + formatter.write_str("a map with one valid filter such as `sender`, `receiver`, `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 { + "sender" => { + filter.sender = map.next_value()?; + }, + //"receiver" => match map.next_value()? { + "receiver" => { + filter.receiver = FilterOperator::ContractCreation; + /* + FilterOperator::Eq(inner_v) => { + filter.receiver = FilterOperator::Eq(Some(inner_v)); + } + FilterOperator::ContractCreation(inner_v) => match inner_v.as_ptr() { + "contract_creation" => filter.receiver = FilterOperator::ContractCreation(None), + _ => { + return Err(M::Error::custom( + "`action` only supports the value `contract_creation`", + )) + } + }, + _ => { + return Err(M::Error::custom( + "the receiver filter only supports the `eq` or `action` operator", + )) + } + */ + }, + "gas" => { + filter.gas = map.next_value()?; + }, + "gas_price" => { + filter.gas_price = map.next_value()?; + }, + "value" => { + filter.value = map.next_value()?; + }, + "nonce" => { + filter.nonce = map.next_value()?; + }, + uf @ _ => { + return Err(M::Error::unknown_field( + uf, + &["sender", "receiver", "gas", "gas_price", "value", "nonce"], + )) + } + } + } + + Ok(filter) + } + } + + impl<'de, T: Deserialize<'de>> Deserialize<'de> for FilterOperator { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct FilterOperatorVisitor { + data: PhantomData, + }; + impl<'de, T: Deserialize<'de>> Visitor<'de> for FilterOperatorVisitor { + type Value = FilterOperator; + + 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 receiver filter can also contain `action`", + ) + } + + fn visit_map(self, mut map: M) -> Result + where + M: MapAccess<'de>, + { + let mut counter = 0; + let mut f_op = FilterOperator::Any; + + while let Some(key) = map.next_key()? { + match key { + "eq" => f_op = FilterOperator::Eq(map.next_value()?), + "gt" => f_op = FilterOperator::GreaterThan(map.next_value()?), + "lt" => f_op = FilterOperator::LessThan(map.next_value()?), + "action" => { + match map.next_value()? { + "contract_creation" => { + f_op = FilterOperator::ContractCreation; + }, + _ => { + return Err(M::Error::custom( + "`action` only supports the value `contract_creation`", + )) + } + } + } + uf @ _ => { + // skip mentioning `action` since it's a special/rare + // case and might confuse the usage with other filters. + return Err(M::Error::unknown_field(uf, &["eq", "gt", "lt"])); + } + } + + 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", + )); + } + + Ok(f_op) + } + } + + deserializer.deserialize_map(FilterOperatorVisitor { data: PhantomData }) + } + } + + deserializer.deserialize_map(FilterOptionsVisitor) + } +} diff --git a/rpc/src/v1/helpers/mod.rs b/rpc/src/v1/helpers/mod.rs index 8a25f93056a..a42fec1e5db 100644 --- a/rpc/src/v1/helpers/mod.rs +++ b/rpc/src/v1/helpers/mod.rs @@ -32,6 +32,7 @@ pub mod nonce; #[cfg(any(test, feature = "accounts"))] pub mod secretstore; +mod filter_options; mod network_settings; mod poll_filter; mod poll_manager; @@ -42,6 +43,7 @@ mod work; mod signature; pub use self::dispatch::{Dispatcher, FullDispatcher, LightDispatcher}; +pub use self::filter_options::{FilterOptions, FilterOperator}; pub use self::signature::verify_signature; pub use self::network_settings::NetworkSettings; pub use self::poll_manager::PollManager; From 806097f8c35024081ccf265f08307b3c70d874ee Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Thu, 20 Jun 2019 21:01:04 +0000 Subject: [PATCH 18/42] implement Validate for MapAccess type --- rpc/src/v1/helpers/filter_options.rs | 92 ++++++++++++++++++++-------- 1 file changed, 67 insertions(+), 25 deletions(-) diff --git a/rpc/src/v1/helpers/filter_options.rs b/rpc/src/v1/helpers/filter_options.rs index cae538be33b..8e0b7633494 100644 --- a/rpc/src/v1/helpers/filter_options.rs +++ b/rpc/src/v1/helpers/filter_options.rs @@ -35,6 +35,67 @@ pub enum FilterOperator { ContractCreation, // only used for `receiver` } +/// Since there are multiple operators which are not supported equally by all filters, +/// this trait will validate each of those. The corresponding method is called inside +/// the `Deserialize` implementation for FilterOperator. In case new operators get +/// introduced, a whitelist instead of a blacklist is used. +/// +/// The `sender` filter validates with `validate_sender` +/// The `receiver` filter validates with `validate_receiver` +/// All other filters such as gas and price validate with `validate_value` +trait Validate<'de, T, M: MapAccess<'de>> { + fn validate_sender(&mut self) -> Result, M::Error>; + fn validate_receiver(&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> +{ + fn validate_sender(&mut self) -> Result, M::Error> { + use self::FilterOperator::*; + let val = self.next_value()?; + match val { + Any => Ok(val), + Eq(_) => Ok(val), + _ => { + Err(M::Error::custom( + "the sender filter only supports the `eq` operator", + )) + } + } + } + fn validate_receiver(&mut self) -> Result, M::Error> { + use self::FilterOperator::*; + let val = self.next_value()?; + match val { + Any => Ok(val), + Eq(_) => Ok(val), + ContractCreation => Ok(val), + _ => { + Err(M::Error::custom( + "the sender filter only supports the `eq` and `action` operators", + )) + } + } + } + fn validate_value(&mut self) -> Result, M::Error> { + use self::FilterOperator::*; + let val = self.next_value()?; + match val { + Any => Ok(val), + Eq(_) => Ok(val), + GreaterThan(_) => Ok(val), + LessThan(_) => Ok(val), + ContractCreation => { + Err(M::Error::custom( + "the operator `action` is only supported by the receiver filter", + )) + } + } + } +} + impl<'de> Deserialize<'de> for FilterOptions { fn deserialize(deserializer: D) -> Result where @@ -57,41 +118,22 @@ impl<'de> Deserialize<'de> for FilterOptions { while let Some(key) = map.next_key()? { match key { "sender" => { - filter.sender = map.next_value()?; + filter.sender = map.validate_sender()?; }, - //"receiver" => match map.next_value()? { "receiver" => { - filter.receiver = FilterOperator::ContractCreation; - /* - FilterOperator::Eq(inner_v) => { - filter.receiver = FilterOperator::Eq(Some(inner_v)); - } - FilterOperator::ContractCreation(inner_v) => match inner_v.as_ptr() { - "contract_creation" => filter.receiver = FilterOperator::ContractCreation(None), - _ => { - return Err(M::Error::custom( - "`action` only supports the value `contract_creation`", - )) - } - }, - _ => { - return Err(M::Error::custom( - "the receiver filter only supports the `eq` or `action` operator", - )) - } - */ + filter.receiver = map.validate_receiver()?; }, "gas" => { - filter.gas = map.next_value()?; + filter.gas = map.validate_value()?; }, "gas_price" => { - filter.gas_price = map.next_value()?; + filter.gas_price = map.validate_value()?; }, "value" => { - filter.value = map.next_value()?; + filter.value = map.validate_value()?; }, "nonce" => { - filter.nonce = map.next_value()?; + filter.nonce = map.validate_value()?; }, uf @ _ => { return Err(M::Error::unknown_field( From 420b1eb5483b0647c4edc577c942ac256f6954f9 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Thu, 20 Jun 2019 21:09:49 +0000 Subject: [PATCH 19/42] additional formatting --- rpc/src/v1/helpers/filter_options.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/rpc/src/v1/helpers/filter_options.rs b/rpc/src/v1/helpers/filter_options.rs index 8e0b7633494..181844aa2a6 100644 --- a/rpc/src/v1/helpers/filter_options.rs +++ b/rpc/src/v1/helpers/filter_options.rs @@ -56,8 +56,7 @@ impl<'de, T, M> Validate<'de, T, M> for M use self::FilterOperator::*; let val = self.next_value()?; match val { - Any => Ok(val), - Eq(_) => Ok(val), + Any | Eq(_) => Ok(val), _ => { Err(M::Error::custom( "the sender filter only supports the `eq` operator", @@ -69,9 +68,7 @@ impl<'de, T, M> Validate<'de, T, M> for M use self::FilterOperator::*; let val = self.next_value()?; match val { - Any => Ok(val), - Eq(_) => Ok(val), - ContractCreation => Ok(val), + Any | Eq(_) | ContractCreation => Ok(val), _ => { Err(M::Error::custom( "the sender filter only supports the `eq` and `action` operators", @@ -83,10 +80,7 @@ impl<'de, T, M> Validate<'de, T, M> for M use self::FilterOperator::*; let val = self.next_value()?; match val { - Any => Ok(val), - Eq(_) => Ok(val), - GreaterThan(_) => Ok(val), - LessThan(_) => Ok(val), + Any | Eq(_) | GreaterThan(_) | LessThan(_) => Ok(val), ContractCreation => { Err(M::Error::custom( "the operator `action` is only supported by the receiver filter", @@ -221,4 +215,4 @@ impl<'de> Deserialize<'de> for FilterOptions { deserializer.deserialize_map(FilterOptionsVisitor) } -} +} \ No newline at end of file From 94560c36748f1abcd97d9b4aced668befd24e1ca Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Thu, 20 Jun 2019 21:12:42 +0000 Subject: [PATCH 20/42] directly name cover in pattern matching --- rpc/src/v1/helpers/filter_options.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rpc/src/v1/helpers/filter_options.rs b/rpc/src/v1/helpers/filter_options.rs index 181844aa2a6..b5b495f51c7 100644 --- a/rpc/src/v1/helpers/filter_options.rs +++ b/rpc/src/v1/helpers/filter_options.rs @@ -129,9 +129,9 @@ impl<'de> Deserialize<'de> for FilterOptions { "nonce" => { filter.nonce = map.validate_value()?; }, - uf @ _ => { + unknown => { return Err(M::Error::unknown_field( - uf, + unknown, &["sender", "receiver", "gas", "gas_price", "value", "nonce"], )) } @@ -185,10 +185,10 @@ impl<'de> Deserialize<'de> for FilterOptions { } } } - uf @ _ => { + 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(uf, &["eq", "gt", "lt"])); + return Err(M::Error::unknown_field(unknown, &["eq", "gt", "lt"])); } } From a303decc101b78ce591102bd3d6bb9d4ade092e1 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Fri, 21 Jun 2019 18:19:13 +0000 Subject: [PATCH 21/42] test valid vull deserialization --- rpc/src/v1/helpers/filter_options.rs | 61 ++++++++++++++++++++++++++-- 1 file changed, 58 insertions(+), 3 deletions(-) diff --git a/rpc/src/v1/helpers/filter_options.rs b/rpc/src/v1/helpers/filter_options.rs index b5b495f51c7..c1f4c7ff53b 100644 --- a/rpc/src/v1/helpers/filter_options.rs +++ b/rpc/src/v1/helpers/filter_options.rs @@ -3,10 +3,10 @@ use serde::de::{Deserialize, Deserializer, Error, MapAccess, Visitor}; use std::fmt; use std::marker::PhantomData; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Eq, PartialEq)] pub struct FilterOptions { sender: FilterOperator
, - receiver: FilterOperator>, + receiver: FilterOperator
, gas: FilterOperator, gas_price: FilterOperator, value: FilterOperator, @@ -26,7 +26,7 @@ impl Default for FilterOptions { } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Eq, PartialEq)] pub enum FilterOperator { Any, Eq(T), @@ -215,4 +215,59 @@ impl<'de> Deserialize<'de> for FilterOptions { deserializer.deserialize_map(FilterOptionsVisitor) } +} + +//#[cfg(test)] +mod tests { + use ethereum_types::{Address, U256}; + use serde_json; + use super::*; + use std::str::FromStr; + + #[test] + fn valid_defaults() { + let res = FilterOptions::default(); + assert_eq!(res.sender, FilterOperator::Any); + assert_eq!(res.receiver, FilterOperator::Any); + assert_eq!(res.gas, FilterOperator::Any); + assert_eq!(res.gas_price, FilterOperator::Any); + assert_eq!(res.value, FilterOperator::Any); + assert_eq!(res.nonce, FilterOperator::Any); + } + + #[test] + fn valid_full_deserialization() { + let json = r#" + { + "sender": { + "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f" + }, + "receiver": { + "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 { + sender: FilterOperator::Eq(Address::from_str("5f3dffcf347944d3739b0805c934d86c8621997f").unwrap()), + receiver: FilterOperator::Eq(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)), + }) + } } \ No newline at end of file From e021a6d21a5db0553d9570e02725d474f0aa79f4 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Fri, 21 Jun 2019 18:29:24 +0000 Subject: [PATCH 22/42] test valid sender operators --- rpc/src/v1/helpers/filter_options.rs | 64 ++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/rpc/src/v1/helpers/filter_options.rs b/rpc/src/v1/helpers/filter_options.rs index c1f4c7ff53b..35fa3c0368c 100644 --- a/rpc/src/v1/helpers/filter_options.rs +++ b/rpc/src/v1/helpers/filter_options.rs @@ -270,4 +270,68 @@ mod tests { nonce: FilterOperator::Eq(U256::from(1399)), }) } + + #[test] + fn valid_sender_deserialization() { + // Only one valid operator for sender + let json = r#" + { + "sender": { + "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + sender: FilterOperator::Eq(Address::from_str("5f3dffcf347944d3739b0805c934d86c8621997f").unwrap()), + ..default + }); + + // Multiple operators are invalid + let json = r#" + { + "sender": { + "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f", + "eq": "0x407d73d8a49eeb85d32cf465507dd71d507100c1" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + + // All other operators are invalid + // Gt + let json = r#" + { + "sender": { + "gt": "0x5f3dffcf347944d3739b0805c934d86c8621997f" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + + // Lt + let json = r#" + { + "sender": { + "lt": "0x5f3dffcf347944d3739b0805c934d86c8621997f" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + + // Action + let json = r#" + { + "sender": { + "action": "contract_creation" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + } } \ No newline at end of file From a8b6761e67cfab8c80d3e673cf2028f7d4561ec8 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Fri, 21 Jun 2019 18:44:37 +0000 Subject: [PATCH 23/42] test valid receiver operators --- rpc/src/v1/helpers/filter_options.rs | 85 +++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 1 deletion(-) diff --git a/rpc/src/v1/helpers/filter_options.rs b/rpc/src/v1/helpers/filter_options.rs index 35fa3c0368c..fbbd4155c32 100644 --- a/rpc/src/v1/helpers/filter_options.rs +++ b/rpc/src/v1/helpers/filter_options.rs @@ -287,7 +287,10 @@ mod tests { sender: FilterOperator::Eq(Address::from_str("5f3dffcf347944d3739b0805c934d86c8621997f").unwrap()), ..default }); + } + #[test] + fn invalid_sender_deserialization() { // Multiple operators are invalid let json = r#" { @@ -300,7 +303,6 @@ mod tests { let res = serde_json::from_str::(json); assert!(res.is_err()); - // All other operators are invalid // Gt let json = r#" { @@ -334,4 +336,85 @@ mod tests { let res = serde_json::from_str::(json); assert!(res.is_err()); } + + #[test] + fn valid_receiver_deserialization() { + // Only two valid operator for receiver + // Eq + let json = r#" + { + "receiver": { + "eq": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" + } + } + "#; + let default = FilterOptions::default(); + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + receiver: FilterOperator::Eq(Address::from_str("e8b2d01ffa0a15736b2370b6e5064f9702c891b6").unwrap()), + ..default.clone() + }); + + // Action + let json = r#" + { + "receiver": { + "action": "contract_creation" + } + } + "#; + let res = serde_json::from_str::(json).unwrap(); + assert_eq!(res, FilterOptions { + receiver: FilterOperator::ContractCreation, + ..default + }); + } + + #[test] + fn invalid_receiver_deserialization() { + // Multiple operators are invalid + let json = r#" + { + "receiver": { + "eq": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6", + "action": "contract_creation" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + + // Gt + let json = r#" + { + "receiver": { + "gt": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + + // Lt + let json = r#" + { + "receiver": { + "lt": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + + // Action (invalid value, must be "contract_creation") + let json = r#" + { + "receiver": { + "action": "some_invalid_value" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + } } \ No newline at end of file From 59aa2fdb890c3ca793e6795d103fd52fe13cf2f2 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Fri, 21 Jun 2019 18:55:54 +0000 Subject: [PATCH 24/42] test valid gas operators --- rpc/src/v1/helpers/filter_options.rs | 62 ++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/rpc/src/v1/helpers/filter_options.rs b/rpc/src/v1/helpers/filter_options.rs index fbbd4155c32..b9ce02df71f 100644 --- a/rpc/src/v1/helpers/filter_options.rs +++ b/rpc/src/v1/helpers/filter_options.rs @@ -417,4 +417,66 @@ mod tests { let res = serde_json::from_str::(json); assert!(res.is_err()); } + + #[test] + fn valid_gas_deserialization() { + // 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() + }); + + // 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.clone() + }); + } + + #[test] + fn invalid_gas_deserialization() { + // Action + let json = r#" + { + "gas": { + "action": "contract_creation" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + } } \ No newline at end of file From f62cf7df93e93a2c01060ff35f1792b63eb7950e Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Fri, 21 Jun 2019 19:01:50 +0000 Subject: [PATCH 25/42] test valid gas price operators --- rpc/src/v1/helpers/filter_options.rs | 62 ++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/rpc/src/v1/helpers/filter_options.rs b/rpc/src/v1/helpers/filter_options.rs index b9ce02df71f..37821eed806 100644 --- a/rpc/src/v1/helpers/filter_options.rs +++ b/rpc/src/v1/helpers/filter_options.rs @@ -479,4 +479,66 @@ mod tests { let res = serde_json::from_str::(json); assert!(res.is_err()); } + + #[test] + fn valid_gas_price_deserialization() { + // 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() + }); + + // 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.clone() + }); + } + + #[test] + fn invalid_gas_price_deserialization() { + // Action + let json = r#" + { + "gas_price": { + "action": "contract_creation" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + } } \ No newline at end of file From 1bb43a781240ab34330c97e38fc9cea870a92782 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Fri, 21 Jun 2019 19:07:15 +0000 Subject: [PATCH 26/42] test valid value operators --- rpc/src/v1/helpers/filter_options.rs | 66 +++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/rpc/src/v1/helpers/filter_options.rs b/rpc/src/v1/helpers/filter_options.rs index 37821eed806..4b8636869d9 100644 --- a/rpc/src/v1/helpers/filter_options.rs +++ b/rpc/src/v1/helpers/filter_options.rs @@ -462,7 +462,7 @@ mod tests { let res = serde_json::from_str::(json).unwrap(); assert_eq!(res, FilterOptions { gas: FilterOperator::LessThan(U256::from(300_000)), - ..default.clone() + ..default }); } @@ -524,7 +524,7 @@ mod tests { 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.clone() + ..default }); } @@ -541,4 +541,66 @@ mod tests { let res = serde_json::from_str::(json); assert!(res.is_err()); } + + #[test] + fn valid_value_deserialization() { + // 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() + }); + + // 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_deserialization() { + // Action + let json = r#" + { + "value": { + "action": "contract_creation" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + } } \ No newline at end of file From f27ae29a3d27766a98047ef62cf796c7a12d422a Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Fri, 21 Jun 2019 19:10:23 +0000 Subject: [PATCH 27/42] test valid nonce operators --- rpc/src/v1/helpers/filter_options.rs | 62 ++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/rpc/src/v1/helpers/filter_options.rs b/rpc/src/v1/helpers/filter_options.rs index 4b8636869d9..adc80216691 100644 --- a/rpc/src/v1/helpers/filter_options.rs +++ b/rpc/src/v1/helpers/filter_options.rs @@ -603,4 +603,66 @@ mod tests { let res = serde_json::from_str::(json); assert!(res.is_err()); } + + #[test] + fn valid_nonce_deserialization() { + // 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() + }); + + // 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_deserialization() { + // Action + let json = r#" + { + "nonce": { + "action": "contract_creation" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); + } } \ No newline at end of file From 9d7684cf89ead010b95ef3dafd91596aad0510f0 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Fri, 21 Jun 2019 19:27:01 +0000 Subject: [PATCH 28/42] additional tsets for defaults, unknown filter types, unknown operators and some renames --- rpc/src/v1/helpers/filter_options.rs | 188 ++++++++++++++++++++++++--- 1 file changed, 168 insertions(+), 20 deletions(-) diff --git a/rpc/src/v1/helpers/filter_options.rs b/rpc/src/v1/helpers/filter_options.rs index adc80216691..9c07d309adb 100644 --- a/rpc/src/v1/helpers/filter_options.rs +++ b/rpc/src/v1/helpers/filter_options.rs @@ -217,7 +217,7 @@ impl<'de> Deserialize<'de> for FilterOptions { } } -//#[cfg(test)] +#[cfg(test)] mod tests { use ethereum_types::{Address, U256}; use serde_json; @@ -226,13 +226,17 @@ mod tests { #[test] fn valid_defaults() { - let res = FilterOptions::default(); - assert_eq!(res.sender, FilterOperator::Any); - assert_eq!(res.receiver, FilterOperator::Any); - assert_eq!(res.gas, FilterOperator::Any); - assert_eq!(res.gas_price, FilterOperator::Any); - assert_eq!(res.value, FilterOperator::Any); - assert_eq!(res.nonce, FilterOperator::Any); + let default = FilterOptions::default(); + assert_eq!(default.sender, FilterOperator::Any); + assert_eq!(default.receiver, 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); } #[test] @@ -272,7 +276,37 @@ mod tests { } #[test] - fn valid_sender_deserialization() { + fn invalid_full_deserialization() { + // Invalid filter type `zyx` + let json = r#" + { + "sender": { + "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f" + }, + "receiver": { + "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()) + } + + #[test] + fn valid_sender_operators() { // Only one valid operator for sender let json = r#" { @@ -290,7 +324,7 @@ mod tests { } #[test] - fn invalid_sender_deserialization() { + fn invalid_sender_operators() { // Multiple operators are invalid let json = r#" { @@ -335,10 +369,21 @@ mod tests { "#; let res = serde_json::from_str::(json); assert!(res.is_err()); + + // Unknown operator + let json = r#" + { + "sender": { + "abc": "0x0" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); } #[test] - fn valid_receiver_deserialization() { + fn valid_receiver_operators() { // Only two valid operator for receiver // Eq let json = r#" @@ -371,7 +416,7 @@ mod tests { } #[test] - fn invalid_receiver_deserialization() { + fn invalid_receiver_operators() { // Multiple operators are invalid let json = r#" { @@ -416,10 +461,21 @@ mod tests { "#; let res = serde_json::from_str::(json); assert!(res.is_err()); + + // Unknown operator + let json = r#" + { + "receiver": { + "abc": "0x0" + } + } + "#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); } #[test] - fn valid_gas_deserialization() { + fn valid_gas_operators() { // Eq let json = r#" { @@ -467,7 +523,19 @@ mod tests { } #[test] - fn invalid_gas_deserialization() { + 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#" { @@ -478,10 +546,21 @@ mod tests { "#; 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_deserialization() { + fn valid_gas_price_operators() { // Eq let json = r#" { @@ -529,7 +608,19 @@ mod tests { } #[test] - fn invalid_gas_price_deserialization() { + 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#" { @@ -540,10 +631,21 @@ mod tests { "#; 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_deserialization() { + fn valid_value_operators() { // Eq let json = r#" { @@ -591,7 +693,19 @@ mod tests { } #[test] - fn invalid_value_deserialization() { + 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#" { @@ -602,10 +716,21 @@ mod tests { "#; 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_deserialization() { + fn valid_nonce_operators() { // Eq let json = r#" { @@ -653,7 +778,19 @@ mod tests { } #[test] - fn invalid_nonce_deserialization() { + 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#" { @@ -664,5 +801,16 @@ mod tests { "#; 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()); } } \ No newline at end of file From 19be31b76a2ebdde25bdcbfa962efbd78e62faf4 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Sun, 23 Jun 2019 06:38:25 +0200 Subject: [PATCH 29/42] move filter_options to ethcore to avoid package cycling --- {rpc/src/v1/helpers => ethcore/src/miner}/filter_options.rs | 1 + ethcore/src/miner/mod.rs | 3 ++- rpc/src/v1/helpers/mod.rs | 2 -- rpc/src/v1/impls/parity.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename {rpc/src/v1/helpers => ethcore/src/miner}/filter_options.rs (99%) diff --git a/rpc/src/v1/helpers/filter_options.rs b/ethcore/src/miner/filter_options.rs similarity index 99% rename from rpc/src/v1/helpers/filter_options.rs rename to ethcore/src/miner/filter_options.rs index 9c07d309adb..1731b9f1fda 100644 --- a/rpc/src/v1/helpers/filter_options.rs +++ b/ethcore/src/miner/filter_options.rs @@ -3,6 +3,7 @@ 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 #[derive(Debug, Clone, Eq, PartialEq)] pub struct FilterOptions { sender: FilterOperator
, diff --git a/ethcore/src/miner/mod.rs b/ethcore/src/miner/mod.rs index 395f07cdf10..7389a08171d 100644 --- a/ethcore/src/miner/mod.rs +++ b/ethcore/src/miner/mod.rs @@ -20,12 +20,13 @@ //! Keeps track of transactions and currently sealed pending block. mod miner; - +mod filter_options; pub mod pool_client; #[cfg(feature = "stratum")] pub mod stratum; pub use self::miner::{Miner, MinerOptions, Penalization, PendingSet, AuthoringParams, Author}; +pub use self::filter_options::FilterOptions; pub use ethcore_miner::local_accounts::LocalAccounts; pub use ethcore_miner::pool::PendingOrdering; diff --git a/rpc/src/v1/helpers/mod.rs b/rpc/src/v1/helpers/mod.rs index a42fec1e5db..8a25f93056a 100644 --- a/rpc/src/v1/helpers/mod.rs +++ b/rpc/src/v1/helpers/mod.rs @@ -32,7 +32,6 @@ pub mod nonce; #[cfg(any(test, feature = "accounts"))] pub mod secretstore; -mod filter_options; mod network_settings; mod poll_filter; mod poll_manager; @@ -43,7 +42,6 @@ mod work; mod signature; pub use self::dispatch::{Dispatcher, FullDispatcher, LightDispatcher}; -pub use self::filter_options::{FilterOptions, FilterOperator}; pub use self::signature::verify_signature; pub use self::network_settings::NetworkSettings; pub use self::poll_manager::PollManager; diff --git a/rpc/src/v1/impls/parity.rs b/rpc/src/v1/impls/parity.rs index 28f8ee3224a..f30dd00963a 100644 --- a/rpc/src/v1/impls/parity.rs +++ b/rpc/src/v1/impls/parity.rs @@ -22,7 +22,7 @@ use std::collections::BTreeMap; use crypto::DEFAULT_MAC; use ethereum_types::{Address, H64, H160, H256, H512, U64, U256}; use ethcore::client::{BlockChainClient, StateClient, Call}; -use ethcore::miner::{self, MinerService}; +use ethcore::miner::{self, MinerService, FilterOptions}; use ethcore::snapshot::{SnapshotService, RestorationStatus}; use ethcore::state::StateInfo; use ethcore_logger::RotatingLogger; From 0719bf3bcae3ff17f134e9d8553305c73db9380f Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Sun, 23 Jun 2019 06:49:29 +0200 Subject: [PATCH 30/42] adjusted function/method parameters for FilterOptions --- ethcore/src/miner/miner.rs | 14 +++++++++----- ethcore/src/miner/mod.rs | 2 +- rpc/src/v1/impls/light/parity.rs | 3 ++- rpc/src/v1/impls/parity.rs | 5 ++--- rpc/src/v1/traits/parity.rs | 3 ++- 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 7bb80047f18..1fc01d70142 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -31,6 +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; use miner::pool_client::{PoolClient, CachedNonceClient, NonceCache}; use miner; use parking_lot::{Mutex, RwLock}; @@ -1052,15 +1053,14 @@ impl miner::MinerService for Miner { C: ChainInfo + Nonce + Sync, { // No special filtering options applied (neither tx_hash, receiver or sender) - self.ready_transactions_filtered(chain, max_len, None, None, ordering) + self.ready_transactions_filtered(chain, max_len, None, ordering) } fn ready_transactions_filtered( &self, chain: &C, max_len: usize, - sender: Option
, - receiver: Option>, + filter: Option, ordering: miner::PendingOrdering, ) -> Vec> where C: ChainInfo + Nonce + Sync, @@ -1093,11 +1093,15 @@ impl miner::MinerService for Miner { .map(Arc::new) // Filter by sender .filter(|tx| { - sender.map_or(true, |sender| tx.signed().sender() == sender) + // TODO + // sender.map_or(true, |sender| tx.signed().sender() == sender) + true }) // Filter by receiver .filter(|tx| { - receiver.map_or(true, |receiver| tx.signed().receiver() == receiver) + // TODO + // receiver.map_or(true, |receiver| tx.signed().receiver() == receiver) + true }) .take(max_len) .collect() diff --git a/ethcore/src/miner/mod.rs b/ethcore/src/miner/mod.rs index 7389a08171d..c6397fccc12 100644 --- a/ethcore/src/miner/mod.rs +++ b/ethcore/src/miner/mod.rs @@ -190,7 +190,7 @@ pub trait MinerService : Send + Sync { /// Depending on the settings may look in transaction pool or only in pending block. /// If you don't need a full set of transactions, you can add `max_len` and create only a limited set of /// transactions. - fn ready_transactions_filtered(&self, chain: &C, max_len: usize, sender: Option
, receiver: Option>, ordering: PendingOrdering) -> Vec> + fn ready_transactions_filtered(&self, chain: &C, max_len: usize, filter: Option, ordering: PendingOrdering) -> Vec> where C: ChainInfo + Nonce + Sync; /// Get a list of all transactions in the pool (some of them might not be ready for inclusion yet). diff --git a/rpc/src/v1/impls/light/parity.rs b/rpc/src/v1/impls/light/parity.rs index 2e390796b58..8849b8e58ad 100644 --- a/rpc/src/v1/impls/light/parity.rs +++ b/rpc/src/v1/impls/light/parity.rs @@ -26,6 +26,7 @@ use ethstore::random_phrase; use sync::{LightSyncInfo, LightSyncProvider, LightNetworkDispatcher, ManageNetwork}; use updater::VersionInfo as UpdaterVersionInfo; use ethereum_types::{H64, H160, H256, H512, U64, U256}; +use ethcore::miner::FilterOptions; use ethcore_logger::RotatingLogger; use jsonrpc_core::{Result, BoxFuture}; @@ -217,7 +218,7 @@ where .map(Into::into) } - fn pending_transactions(&self, limit: Option, _sender: Option, _receiver: Option>) -> Result> { + fn pending_transactions(&self, limit: Option, _sender: Option) -> Result> { let txq = self.light_dispatch.transaction_queue.read(); let chain_info = self.light_dispatch.client.chain_info(); Ok( diff --git a/rpc/src/v1/impls/parity.rs b/rpc/src/v1/impls/parity.rs index f30dd00963a..1d5556f4381 100644 --- a/rpc/src/v1/impls/parity.rs +++ b/rpc/src/v1/impls/parity.rs @@ -245,12 +245,11 @@ impl Parity for ParityClient where .map(Into::into) } - fn pending_transactions(&self, limit: Option, sender: Option
, receiver: Option>) -> Result> { + fn pending_transactions(&self, limit: Option, filter: Option) -> Result> { let ready_transactions = self.miner.ready_transactions_filtered( &*self.client, limit.unwrap_or_else(usize::max_value), - sender, - receiver, + filter, miner::PendingOrdering::Priority, ); diff --git a/rpc/src/v1/traits/parity.rs b/rpc/src/v1/traits/parity.rs index bc37da789cb..050c9bd07fa 100644 --- a/rpc/src/v1/traits/parity.rs +++ b/rpc/src/v1/traits/parity.rs @@ -19,6 +19,7 @@ use std::collections::BTreeMap; use ethereum_types::{H64, H160, H256, H512, U64, U256}; +use ethcore::miner::FilterOptions; use jsonrpc_core::{BoxFuture, Result}; use jsonrpc_derive::rpc; use v1::types::{ @@ -125,7 +126,7 @@ pub trait Parity { /// Returns all pending transactions from transaction queue. #[rpc(name = "parity_pendingTransactions")] - fn pending_transactions(&self, Option, Option, Option>) -> Result>; + fn pending_transactions(&self, Option, Option) -> Result>; /// Returns all transactions from transaction queue. /// From c1d94c7cbd2df7c4712dc82fa8f1150f99457ba1 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Sun, 23 Jun 2019 21:50:05 +0200 Subject: [PATCH 31/42] implement filter for sender and receiver --- ethcore/src/miner/filter_options.rs | 12 ++++++------ ethcore/src/miner/miner.rs | 26 ++++++++++++++++++++------ 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/ethcore/src/miner/filter_options.rs b/ethcore/src/miner/filter_options.rs index 1731b9f1fda..2ef610299aa 100644 --- a/ethcore/src/miner/filter_options.rs +++ b/ethcore/src/miner/filter_options.rs @@ -6,12 +6,12 @@ use std::marker::PhantomData; /// This structure provides filtering options for the pending transactions RPC API call #[derive(Debug, Clone, Eq, PartialEq)] pub struct FilterOptions { - sender: FilterOperator
, - receiver: FilterOperator
, - gas: FilterOperator, - gas_price: FilterOperator, - value: FilterOperator, - nonce: FilterOperator, + pub sender: FilterOperator
, + pub receiver: FilterOperator
, + pub gas: FilterOperator, + pub gas_price: FilterOperator, + pub value: FilterOperator, + pub nonce: FilterOperator, } impl Default for FilterOptions { diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 1fc01d70142..15bbc84dffb 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1085,6 +1085,7 @@ impl miner::MinerService for Miner { ) }; + use miner::filter_options::FilterOperator::*; let from_pending = || { self.map_existing_pending_block(|sealing| { sealing.transactions @@ -1093,15 +1094,28 @@ impl miner::MinerService for Miner { .map(Arc::new) // Filter by sender .filter(|tx| { - // TODO - // sender.map_or(true, |sender| tx.signed().sender() == sender) - true + if let Some(ref filter) = filter { + let sender = tx.signed().sender(); + match filter.sender { + Eq(value) => sender == value, + _ => true, + } + } else { + true + } }) // Filter by receiver .filter(|tx| { - // TODO - // receiver.map_or(true, |receiver| tx.signed().receiver() == receiver) - true + if let Some(ref filter) = filter { + let receiver = tx.signed().receiver(); + match filter.receiver { + Eq(value) => receiver == Some(value), + ContractCreation => receiver == None, + _ => true, + } + } else { + true + } }) .take(max_len) .collect() From 36b9b3c165e8706e4da50440990396da78089ff2 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Sun, 23 Jun 2019 21:58:15 +0200 Subject: [PATCH 32/42] implement filter for gas --- ethcore/src/miner/miner.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 15bbc84dffb..1c114b0b2b8 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1117,6 +1117,20 @@ impl miner::MinerService for Miner { true } }) + // Filter by gas + .filter(|tx| { + if let Some(ref filter) = filter { + let gas = tx.signed().as_unsigned().gas; + match filter.gas { + Eq(value) => gas == value, + GreaterThan(value) => gas > value, + LessThan(value) => gas < value, + _ => true, + } + } else { + true + } + }) .take(max_len) .collect() }, chain_info.best_block_number) From 8542e5bf662d44b30f2ce6a4b0cedb4cd6ef1134 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Sun, 23 Jun 2019 22:19:13 +0200 Subject: [PATCH 33/42] implement filter for gas price, tx value and nonce --- ethcore/src/miner/miner.rs | 48 +++++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 1c114b0b2b8..488b3c23107 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1101,7 +1101,7 @@ impl miner::MinerService for Miner { _ => true, } } else { - true + true // no filter } }) // Filter by receiver @@ -1114,7 +1114,7 @@ impl miner::MinerService for Miner { _ => true, } } else { - true + true // no filter } }) // Filter by gas @@ -1128,7 +1128,49 @@ impl miner::MinerService for Miner { _ => true, } } else { - true + true // no filter + } + }) + // Filter by gas price + .filter(|tx| { + if let Some(ref filter) = filter { + let gas_price = tx.signed().as_unsigned().gas_price; + match filter.gas_price { + Eq(value) => gas_price == value, + GreaterThan(value) => gas_price > value, + LessThan(value) => gas_price < value, + _ => true, + } + } else { + true // no filter + } + }) + // Filter by tx value + .filter(|tx| { + if let Some(ref filter) = filter { + let tx_value = tx.signed().as_unsigned().value; + match filter.value { + Eq(value) => tx_value == value, + GreaterThan(value) => tx_value > value, + LessThan(value) => tx_value < value, + _ => true, + } + } else { + true // no filter + } + }) + // Filter by nonce + .filter(|tx| { + if let Some(ref filter) = filter { + let nonce = tx.signed().as_unsigned().nonce; + match filter.nonce { + Eq(value) => nonce == value, + GreaterThan(value) => nonce > value, + LessThan(value) => nonce < value, + _ => true, + } + } else { + true // no filter } }) .take(max_len) From 3af058f6de13de6799c56256e5933426b18be273 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Mon, 24 Jun 2019 18:35:49 +0000 Subject: [PATCH 34/42] improve filtering implementation; use common function, use combinators --- ethcore/src/miner/miner.rs | 84 ++++++++++++++------------------------ 1 file changed, 31 insertions(+), 53 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 488b3c23107..9b22cddcf05 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; +use miner::filter_options::{FilterOptions, FilterOperator}; use miner::pool_client::{PoolClient, CachedNonceClient, NonceCache}; use miner; use parking_lot::{Mutex, RwLock}; @@ -1088,90 +1088,68 @@ impl miner::MinerService for Miner { 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 occure on `Any`, other operators + // get filtered out during deserialization + _ => true, + } + } + sealing.transactions .iter() .map(|signed| pool::VerifiedTransaction::from_pending_block_transaction(signed.clone())) .map(Arc::new) // Filter by sender .filter(|tx| { - if let Some(ref filter) = filter { + filter.as_ref().map_or(true, |filter| { let sender = tx.signed().sender(); match filter.sender { Eq(value) => sender == value, _ => true, } - } else { - true // no filter - } + }) }) // Filter by receiver .filter(|tx| { - if let Some(ref filter) = filter { + filter.as_ref().map_or(true, |filter| { let receiver = tx.signed().receiver(); match filter.receiver { Eq(value) => receiver == Some(value), ContractCreation => receiver == None, _ => true, } - } else { - true // no filter - } + }) }) // Filter by gas .filter(|tx| { - if let Some(ref filter) = filter { - let gas = tx.signed().as_unsigned().gas; - match filter.gas { - Eq(value) => gas == value, - GreaterThan(value) => gas > value, - LessThan(value) => gas < value, - _ => true, - } - } else { - true // no filter - } + filter.as_ref().map_or(true, |filter| { + match_common_filter(&filter.gas, &tx.signed().as_unsigned().gas) + }) }) // Filter by gas price .filter(|tx| { - if let Some(ref filter) = filter { - let gas_price = tx.signed().as_unsigned().gas_price; - match filter.gas_price { - Eq(value) => gas_price == value, - GreaterThan(value) => gas_price > value, - LessThan(value) => gas_price < value, - _ => true, - } - } else { - true // no filter - } + filter.as_ref().map_or(true, |filter| { + match_common_filter(&filter.gas_price, &tx.signed().as_unsigned().gas_price) + }) }) // Filter by tx value .filter(|tx| { - if let Some(ref filter) = filter { - let tx_value = tx.signed().as_unsigned().value; - match filter.value { - Eq(value) => tx_value == value, - GreaterThan(value) => tx_value > value, - LessThan(value) => tx_value < value, - _ => true, - } - } else { - true // no filter - } + filter.as_ref().map_or(true, |filter| { + match_common_filter(&filter.value, &tx.signed().as_unsigned().value) + }) }) // Filter by nonce .filter(|tx| { - if let Some(ref filter) = filter { - let nonce = tx.signed().as_unsigned().nonce; - match filter.nonce { - Eq(value) => nonce == value, - GreaterThan(value) => nonce > value, - LessThan(value) => nonce < value, - _ => true, - } - } else { - true // no filter - } + filter.as_ref().map_or(true, |filter| { + match_common_filter(&filter.nonce, &tx.signed().as_unsigned().nonce) + }) }) .take(max_len) .collect() From ce2eef1511c9a0563b06e727a7e7d55794b5576c Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Mon, 24 Jun 2019 18:41:01 +0000 Subject: [PATCH 35/42] improved documentation for FilterOptions --- ethcore/src/miner/filter_options.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/ethcore/src/miner/filter_options.rs b/ethcore/src/miner/filter_options.rs index 2ef610299aa..0c43cc52bcd 100644 --- a/ethcore/src/miner/filter_options.rs +++ b/ethcore/src/miner/filter_options.rs @@ -6,11 +6,17 @@ use std::marker::PhantomData; /// This structure provides filtering options for the pending transactions RPC API call #[derive(Debug, Clone, Eq, PartialEq)] pub struct FilterOptions { + // Contains the operator to filter the sender value of the transaction pub sender: FilterOperator
, + // Contains the operator to filter the receiver value of the transaction pub receiver: 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, } @@ -37,9 +43,9 @@ pub enum FilterOperator { } /// Since there are multiple operators which are not supported equally by all filters, -/// this trait will validate each of those. The corresponding method is called inside -/// the `Deserialize` implementation for FilterOperator. In case new operators get -/// introduced, a whitelist instead of a blacklist is used. +/// this trait will validate each of those operators. The corresponding method is called +/// inside the `Deserialize` -> `Visitor` implementation for FilterOperator. In case new +/// operators get introduced, a whitelist instead of a blacklist is used. /// /// The `sender` filter validates with `validate_sender` /// The `receiver` filter validates with `validate_receiver` @@ -331,7 +337,7 @@ mod tests { { "sender": { "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f", - "eq": "0x407d73d8a49eeb85d32cf465507dd71d507100c1" + "lt": "0x407d73d8a49eeb85d32cf465507dd71d507100c1" } } "#; From c977666181dbf7c1a7191d6c83140497661de76d Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Mon, 24 Jun 2019 18:56:43 +0000 Subject: [PATCH 36/42] small documentation adjustments --- ethcore/src/miner/miner.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 9b22cddcf05..7c4775a428e 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1097,7 +1097,7 @@ impl miner::MinerService for Miner { GreaterThan(value) => tx_value > value, LessThan(value) => tx_value < value, // Will always occure on `Any`, other operators - // get filtered out during deserialization + // get handled during deserialization _ => true, } } @@ -1112,6 +1112,8 @@ impl miner::MinerService for Miner { let sender = tx.signed().sender(); match filter.sender { Eq(value) => sender == value, + // Will always occure on `Any`, other operators + // get handled during deserialization _ => true, } }) @@ -1123,6 +1125,8 @@ impl miner::MinerService for Miner { match filter.receiver { Eq(value) => receiver == Some(value), ContractCreation => receiver == None, + // Will always occure on `Any`, other operators + // get handled during deserialization _ => true, } }) From 02a219f15af8c867d2c03ba8650359ac260e223c Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Mon, 24 Jun 2019 19:27:34 +0000 Subject: [PATCH 37/42] remove warnings --- Cargo.lock | 1 + ethcore/Cargo.toml | 1 + ethcore/src/lib.rs | 2 ++ ethcore/src/miner/filter_options.rs | 12 ++++++------ rpc/src/v1/tests/helpers/miner_service.rs | 4 ++-- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c3a00e53ff4..159141096a3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -906,6 +906,7 @@ dependencies = [ "rustc-hex 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.89 (registry+https://github.com/rust-lang/crates.io-index)", "serde_derive 1.0.89 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_json 1.0.39 (registry+https://github.com/rust-lang/crates.io-index)", "stats 0.1.0", "tempdir 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)", "time-utils 0.1.0", diff --git a/ethcore/Cargo.toml b/ethcore/Cargo.toml index 92d3bf3e2c7..566947e8915 100644 --- a/ethcore/Cargo.toml +++ b/ethcore/Cargo.toml @@ -61,6 +61,7 @@ rlp = "0.4.0" rlp_derive = { path = "../util/rlp-derive" } rustc-hex = "1.0" serde = "1.0" +serde_json = "1.0" serde_derive = "1.0" stats = { path = "../util/stats" } tempdir = { version = "0.3", optional = true } diff --git a/ethcore/src/lib.rs b/ethcore/src/lib.rs index 315cd0bb5ed..eec1ba11c7a 100644 --- a/ethcore/src/lib.rs +++ b/ethcore/src/lib.rs @@ -123,6 +123,8 @@ extern crate blooms_db; extern crate env_logger; #[cfg(test)] extern crate rlp_compress; +#[cfg(test)] +extern crate serde_json; #[macro_use] extern crate ethabi_derive; diff --git a/ethcore/src/miner/filter_options.rs b/ethcore/src/miner/filter_options.rs index 0c43cc52bcd..fc20b644172 100644 --- a/ethcore/src/miner/filter_options.rs +++ b/ethcore/src/miner/filter_options.rs @@ -6,17 +6,17 @@ use std::marker::PhantomData; /// This structure provides filtering options for the pending transactions RPC API call #[derive(Debug, Clone, Eq, PartialEq)] pub struct FilterOptions { - // Contains the operator to filter the sender value of the transaction + /// Contains the operator to filter the sender value of the transaction pub sender: FilterOperator
, - // Contains the operator to filter the receiver value of the transaction + /// Contains the operator to filter the receiver value of the transaction pub receiver: FilterOperator
, - // Contains the operator to filter the gas value of the transaction + /// 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 + /// Contains the operator to filter the gas price value of the transaction pub gas_price: FilterOperator, - // Contains the operator to filter the transaction value + /// Contains the operator to filter the transaction value pub value: FilterOperator, - // Contains the operator to filter the nonce value of the transaction + /// Contains the operator to filter the nonce value of the transaction pub nonce: FilterOperator, } diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index 68b7c33bc42..eb06871ff6c 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -24,7 +24,7 @@ use ethcore::block::SealedBlock; use ethcore::client::{Nonce, PrepareOpenBlock, StateClient, EngineInfo}; use ethcore::engines::{EthEngine, signer::EngineSigner}; use ethcore::error::Error; -use ethcore::miner::{self, MinerService, AuthoringParams}; +use ethcore::miner::{self, MinerService, AuthoringParams, FilterOptions}; use ethereum_types::{H256, U256, Address}; use miner::pool::local_transactions::Status as LocalTransactionStatus; use miner::pool::{verifier, VerifiedTransaction, QueueStatus}; @@ -222,7 +222,7 @@ impl MinerService for TestMinerService { self.queued_transactions() } - fn ready_transactions_filtered(&self, _chain: &C, _max_len: usize, _sender: Option
, _receiver: Option>, _ordering: miner::PendingOrdering) -> Vec> { + fn ready_transactions_filtered(&self, _chain: &C, _max_len: usize, _filter: Option, _ordering: miner::PendingOrdering) -> Vec> { self.queued_transactions() } From 9754f1a67d87643497badf8b1209983ec7ccbf3c Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Wed, 26 Jun 2019 20:37:06 +0000 Subject: [PATCH 38/42] replace FilterOperator::ContractCreation with FilterOperator::Eq(None) --- ethcore/src/miner/filter_options.rs | 99 +++++++++++++++++------------ ethcore/src/miner/miner.rs | 4 +- 2 files changed, 61 insertions(+), 42 deletions(-) diff --git a/ethcore/src/miner/filter_options.rs b/ethcore/src/miner/filter_options.rs index fc20b644172..9c836bd7812 100644 --- a/ethcore/src/miner/filter_options.rs +++ b/ethcore/src/miner/filter_options.rs @@ -9,7 +9,7 @@ pub struct FilterOptions { /// Contains the operator to filter the sender value of the transaction pub sender: FilterOperator
, /// Contains the operator to filter the receiver value of the transaction - pub receiver: FilterOperator
, + pub receiver: 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 @@ -33,13 +33,21 @@ impl Default for FilterOptions { } } +/// The highly generic use of implementing Deserialize for FilterOperator +/// will result in a compiler error if the type FilterOperator::Eq(None) +/// gets returned explicitly. Therefore this Wrapper will be used for +/// deserialization, directly identifying the contract creation. +enum Wrapper { + O(FilterOperator), + CC, // Contract Creation +} + #[derive(Debug, Clone, Eq, PartialEq)] pub enum FilterOperator { Any, Eq(T), GreaterThan(T), LessThan(T), - ContractCreation, // only used for `receiver` } /// Since there are multiple operators which are not supported equally by all filters, @@ -47,48 +55,46 @@ pub enum FilterOperator { /// inside the `Deserialize` -> `Visitor` implementation for FilterOperator. In case new /// operators get introduced, a whitelist instead of a blacklist is used. /// -/// The `sender` filter validates with `validate_sender` -/// The `receiver` filter validates with `validate_receiver` +/// The `sender` and `receiver` filter validate with `validate_account` +/// Additionally, the `receiver` filter also contains a separate, +/// custom validator in order to identify the `action` (Wrapper::CC) filter. /// All other filters such as gas and price validate with `validate_value` trait Validate<'de, T, M: MapAccess<'de>> { - fn validate_sender(&mut self) -> Result, M::Error>; - fn validate_receiver(&mut self) -> Result, M::Error>; + fn validate_account(&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> { - fn validate_sender(&mut self) -> Result, M::Error> { + fn validate_account(&mut self) -> Result, M::Error> { + use self::Wrapper as W; use self::FilterOperator::*; - let val = self.next_value()?; - match val { - Any | Eq(_) => Ok(val), - _ => { + let wrapper = self.next_value()?; + match wrapper { + W::O(val) => { + match val { + Any | Eq(_) => Ok(val), + _ => { + Err(M::Error::custom( + "the sender filter only supports the `eq` operator", + )) + } + } + }, + W::CC => { Err(M::Error::custom( "the sender filter only supports the `eq` operator", )) } } } - fn validate_receiver(&mut self) -> Result, M::Error> { - use self::FilterOperator::*; - let val = self.next_value()?; - match val { - Any | Eq(_) | ContractCreation => Ok(val), - _ => { - Err(M::Error::custom( - "the sender filter only supports the `eq` and `action` operators", - )) - } - } - } fn validate_value(&mut self) -> Result, M::Error> { - use self::FilterOperator::*; - let val = self.next_value()?; - match val { - Any | Eq(_) | GreaterThan(_) | LessThan(_) => Ok(val), - ContractCreation => { + 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 receiver filter", )) @@ -119,10 +125,22 @@ impl<'de> Deserialize<'de> for FilterOptions { while let Some(key) = map.next_key()? { match key { "sender" => { - filter.sender = map.validate_sender()?; + filter.sender = map.validate_account()?; }, "receiver" => { - filter.receiver = map.validate_receiver()?; + filter.receiver = match map.next_value()? { + Wrapper::O::
(_) => map + // This filter accepts the same operators as `sender` + // (except for contract creation, handled below), + // therefore the same validator can be called on this. + .validate_account() + .map_err(|_| { + M::Error::custom( + "the receiver filter only supports the `eq` or `action` operator", + ) + })?, + Wrapper::CC => FilterOperator::Eq(None), + } }, "gas" => { filter.gas = map.validate_value()?; @@ -149,16 +167,16 @@ impl<'de> Deserialize<'de> for FilterOptions { } } - impl<'de, T: Deserialize<'de>> Deserialize<'de> for FilterOperator { + impl<'de, T: Deserialize<'de>> Deserialize<'de> for Wrapper { fn deserialize(deserializer: D) -> Result where D: Deserializer<'de>, { - struct FilterOperatorVisitor { + struct WrapperVisitor { data: PhantomData, }; - impl<'de, T: Deserialize<'de>> Visitor<'de> for FilterOperatorVisitor { - type Value = FilterOperator; + 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 ..." @@ -172,18 +190,19 @@ impl<'de> Deserialize<'de> for FilterOptions { where M: MapAccess<'de>, { + use self::Wrapper as W; let mut counter = 0; - let mut f_op = FilterOperator::Any; + let mut f_op = Wrapper::O(FilterOperator::Any); while let Some(key) = map.next_key()? { match key { - "eq" => f_op = FilterOperator::Eq(map.next_value()?), - "gt" => f_op = FilterOperator::GreaterThan(map.next_value()?), - "lt" => f_op = FilterOperator::LessThan(map.next_value()?), + "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 = FilterOperator::ContractCreation; + f_op = W::CC; }, _ => { return Err(M::Error::custom( @@ -216,7 +235,7 @@ impl<'de> Deserialize<'de> for FilterOptions { } } - deserializer.deserialize_map(FilterOperatorVisitor { data: PhantomData }) + deserializer.deserialize_map(WrapperVisitor { data: PhantomData }) } } diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 7c4775a428e..94b8b47f200 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1123,8 +1123,8 @@ impl miner::MinerService for Miner { filter.as_ref().map_or(true, |filter| { let receiver = tx.signed().receiver(); match filter.receiver { - Eq(value) => receiver == Some(value), - ContractCreation => receiver == None, + // Could apply to `Some(Address)` or `None` (for contract creation) + Eq(value) => receiver == value, // Will always occure on `Any`, other operators // get handled during deserialization _ => true, From bd249e54ca8451774f10a7e881705b3ae4781dd1 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Wed, 26 Jun 2019 21:06:36 +0000 Subject: [PATCH 39/42] implement validate_receiver --- ethcore/src/miner/filter_options.rs | 49 ++++++++++++++++------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/ethcore/src/miner/filter_options.rs b/ethcore/src/miner/filter_options.rs index 9c836bd7812..144c006c599 100644 --- a/ethcore/src/miner/filter_options.rs +++ b/ethcore/src/miner/filter_options.rs @@ -55,19 +55,19 @@ pub enum FilterOperator { /// inside the `Deserialize` -> `Visitor` implementation for FilterOperator. In case new /// operators get introduced, a whitelist instead of a blacklist is used. /// -/// The `sender` and `receiver` filter validate with `validate_account` -/// Additionally, the `receiver` filter also contains a separate, -/// custom validator in order to identify the `action` (Wrapper::CC) filter. +/// The `sender` filter validates with `validate_sender` +/// The `receiver` filter validates with `validate_sender` /// All other filters such as gas and price validate with `validate_value` trait Validate<'de, T, M: MapAccess<'de>> { - fn validate_account(&mut self) -> Result, M::Error>; + fn validate_sender(&mut self) -> Result, M::Error>; + fn validate_receiver(&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> { - fn validate_account(&mut self) -> Result, M::Error> { + fn validate_sender(&mut self) -> Result, M::Error> { use self::Wrapper as W; use self::FilterOperator::*; let wrapper = self.next_value()?; @@ -89,6 +89,25 @@ impl<'de, T, M> Validate<'de, T, M> for M } } } + fn validate_receiver(&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 receiver 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()?; @@ -125,22 +144,10 @@ impl<'de> Deserialize<'de> for FilterOptions { while let Some(key) = map.next_key()? { match key { "sender" => { - filter.sender = map.validate_account()?; + filter.sender = map.validate_sender()?; }, "receiver" => { - filter.receiver = match map.next_value()? { - Wrapper::O::
(_) => map - // This filter accepts the same operators as `sender` - // (except for contract creation, handled below), - // therefore the same validator can be called on this. - .validate_account() - .map_err(|_| { - M::Error::custom( - "the receiver filter only supports the `eq` or `action` operator", - ) - })?, - Wrapper::CC => FilterOperator::Eq(None), - } + Validate::<(), _>::validate_receiver(&mut map)?; }, "gas" => { filter.gas = map.validate_value()?; @@ -293,7 +300,7 @@ mod tests { let res = serde_json::from_str::(json).unwrap(); assert_eq!(res, FilterOptions { sender: FilterOperator::Eq(Address::from_str("5f3dffcf347944d3739b0805c934d86c8621997f").unwrap()), - receiver: FilterOperator::Eq(Address::from_str("e8b2d01ffa0a15736b2370b6e5064f9702c891b6").unwrap()), + receiver: 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)), @@ -436,7 +443,7 @@ mod tests { "#; let res = serde_json::from_str::(json).unwrap(); assert_eq!(res, FilterOptions { - receiver: FilterOperator::ContractCreation, + receiver: FilterOperator::Eq(None), ..default }); } From 7a2e942631695d209730fec6f9b5a86042cf81bb Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Wed, 26 Jun 2019 21:36:24 +0000 Subject: [PATCH 40/42] make small changes like renames, preamble --- ethcore/Cargo.toml | 2 +- ethcore/src/miner/filter_options.rs | 123 ++++++++++++++++------------ ethcore/src/miner/miner.rs | 4 +- 3 files changed, 73 insertions(+), 56 deletions(-) diff --git a/ethcore/Cargo.toml b/ethcore/Cargo.toml index 566947e8915..3584f08ee53 100644 --- a/ethcore/Cargo.toml +++ b/ethcore/Cargo.toml @@ -61,7 +61,6 @@ rlp = "0.4.0" rlp_derive = { path = "../util/rlp-derive" } rustc-hex = "1.0" serde = "1.0" -serde_json = "1.0" serde_derive = "1.0" stats = { path = "../util/stats" } tempdir = { version = "0.3", optional = true } @@ -83,6 +82,7 @@ fetch = { path = "../util/fetch" } kvdb-rocksdb = "0.1.3" parity-runtime = { path = "../util/runtime" } rlp_compress = { path = "../util/rlp-compress" } +serde_json = "1.0" tempdir = "0.3" trie-standardmap = "0.12.3" diff --git a/ethcore/src/miner/filter_options.rs b/ethcore/src/miner/filter_options.rs index 144c006c599..0b190096b3b 100644 --- a/ethcore/src/miner/filter_options.rs +++ b/ethcore/src/miner/filter_options.rs @@ -1,3 +1,19 @@ +// Copyright 2015-2019 Parity Technologies (UK) Ltd. +// This file is part of Parity Ethereum. + +// Parity Ethereum is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity Ethereum is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// 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; @@ -6,10 +22,10 @@ use std::marker::PhantomData; /// This structure provides filtering options for the pending transactions RPC API call #[derive(Debug, Clone, Eq, PartialEq)] pub struct FilterOptions { - /// Contains the operator to filter the sender value of the transaction - pub sender: FilterOperator
, - /// Contains the operator to filter the receiver value of the transaction - pub receiver: FilterOperator>, + /// 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 @@ -23,8 +39,8 @@ pub struct FilterOptions { impl Default for FilterOptions { fn default() -> Self { FilterOptions { - sender: FilterOperator::Any, - receiver: FilterOperator::Any, + from: FilterOperator::Any, + to: FilterOperator::Any, gas: FilterOperator::Any, gas_price: FilterOperator::Any, value: FilterOperator::Any, @@ -55,19 +71,19 @@ pub enum FilterOperator { /// inside the `Deserialize` -> `Visitor` implementation for FilterOperator. In case new /// operators get introduced, a whitelist instead of a blacklist is used. /// -/// The `sender` filter validates with `validate_sender` -/// The `receiver` filter validates with `validate_sender` +/// The `from` filter validates with `validate_from` +/// 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_sender(&mut self) -> Result, M::Error>; - fn validate_receiver(&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> { - fn validate_sender(&mut self) -> Result, M::Error> { + fn validate_from(&mut self) -> Result, M::Error> { use self::Wrapper as W; use self::FilterOperator::*; let wrapper = self.next_value()?; @@ -77,19 +93,19 @@ impl<'de, T, M> Validate<'de, T, M> for M Any | Eq(_) => Ok(val), _ => { Err(M::Error::custom( - "the sender filter only supports the `eq` operator", + "the from filter only supports the `eq` operator", )) } } }, W::CC => { Err(M::Error::custom( - "the sender filter only supports the `eq` operator", + "the from filter only supports the `eq` operator", )) } } } - fn validate_receiver(&mut self) -> Result>, M::Error> { + fn validate_to(&mut self) -> Result>, M::Error> { use self::Wrapper as W; use self::FilterOperator::*; let wrapper = self.next_value()?; @@ -100,7 +116,7 @@ impl<'de, T, M> Validate<'de, T, M> for M Eq(address) => Ok(Eq(Some(address))), _ => { Err(M::Error::custom( - "the receiver filter only supports the `eq` or `action` operator", + "the to filter only supports the `eq` or `action` operator", )) } } @@ -115,7 +131,7 @@ impl<'de, T, M> Validate<'de, T, M> for M W::O(val) => Ok(val), W::CC => { Err(M::Error::custom( - "the operator `action` is only supported by the receiver filter", + "the operator `action` is only supported by the to filter", )) } } @@ -133,7 +149,7 @@ impl<'de> Deserialize<'de> for 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 `sender`, `receiver`, `gas`, `gas_price`, `value` or `nonce`") + 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 @@ -143,11 +159,12 @@ impl<'de> Deserialize<'de> for FilterOptions { let mut filter = FilterOptions::default(); while let Some(key) = map.next_key()? { match key { - "sender" => { - filter.sender = map.validate_sender()?; + "from" => { + filter.from = map.validate_from()?; }, - "receiver" => { - Validate::<(), _>::validate_receiver(&mut map)?; + "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()?; @@ -164,7 +181,7 @@ impl<'de> Deserialize<'de> for FilterOptions { unknown => { return Err(M::Error::unknown_field( unknown, - &["sender", "receiver", "gas", "gas_price", "value", "nonce"], + &["from", "to", "gas", "gas_price", "value", "nonce"], )) } } @@ -189,7 +206,7 @@ impl<'de> Deserialize<'de> for FilterOptions { // "This Visitor expects to receive ..." formatter.write_str( "a map with one valid operator such as `eq`, `gt` or `lt`. \ - The receiver filter can also contain `action`", + The to filter can also contain `action`", ) } @@ -260,8 +277,8 @@ mod tests { #[test] fn valid_defaults() { let default = FilterOptions::default(); - assert_eq!(default.sender, FilterOperator::Any); - assert_eq!(default.receiver, FilterOperator::Any); + 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); @@ -276,10 +293,10 @@ mod tests { fn valid_full_deserialization() { let json = r#" { - "sender": { + "from": { "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f" }, - "receiver": { + "to": { "eq": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" }, "gas": { @@ -299,8 +316,8 @@ mod tests { let res = serde_json::from_str::(json).unwrap(); assert_eq!(res, FilterOptions { - sender: FilterOperator::Eq(Address::from_str("5f3dffcf347944d3739b0805c934d86c8621997f").unwrap()), - receiver: FilterOperator::Eq(Some(Address::from_str("e8b2d01ffa0a15736b2370b6e5064f9702c891b6")).unwrap()), + 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)), @@ -313,10 +330,10 @@ mod tests { // Invalid filter type `zyx` let json = r#" { - "sender": { + "from": { "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f" }, - "receiver": { + "to": { "eq": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" }, "zyx": { @@ -339,11 +356,11 @@ mod tests { } #[test] - fn valid_sender_operators() { - // Only one valid operator for sender + fn valid_from_operators() { + // Only one valid operator for from let json = r#" { - "sender": { + "from": { "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f" } } @@ -351,17 +368,17 @@ mod tests { let default = FilterOptions::default(); let res = serde_json::from_str::(json).unwrap(); assert_eq!(res, FilterOptions { - sender: FilterOperator::Eq(Address::from_str("5f3dffcf347944d3739b0805c934d86c8621997f").unwrap()), + from: FilterOperator::Eq(Address::from_str("5f3dffcf347944d3739b0805c934d86c8621997f").unwrap()), ..default }); } #[test] - fn invalid_sender_operators() { + fn invalid_from_operators() { // Multiple operators are invalid let json = r#" { - "sender": { + "from": { "eq": "0x5f3dffcf347944d3739b0805c934d86c8621997f", "lt": "0x407d73d8a49eeb85d32cf465507dd71d507100c1" } @@ -373,7 +390,7 @@ mod tests { // Gt let json = r#" { - "sender": { + "from": { "gt": "0x5f3dffcf347944d3739b0805c934d86c8621997f" } } @@ -384,7 +401,7 @@ mod tests { // Lt let json = r#" { - "sender": { + "from": { "lt": "0x5f3dffcf347944d3739b0805c934d86c8621997f" } } @@ -395,7 +412,7 @@ mod tests { // Action let json = r#" { - "sender": { + "from": { "action": "contract_creation" } } @@ -406,7 +423,7 @@ mod tests { // Unknown operator let json = r#" { - "sender": { + "from": { "abc": "0x0" } } @@ -416,12 +433,12 @@ mod tests { } #[test] - fn valid_receiver_operators() { - // Only two valid operator for receiver + fn valid_to_operators() { + // Only two valid operator for to // Eq let json = r#" { - "receiver": { + "to": { "eq": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" } } @@ -429,31 +446,31 @@ mod tests { let default = FilterOptions::default(); let res = serde_json::from_str::(json).unwrap(); assert_eq!(res, FilterOptions { - receiver: FilterOperator::Eq(Address::from_str("e8b2d01ffa0a15736b2370b6e5064f9702c891b6").unwrap()), + to: FilterOperator::Eq(Some(Address::from_str("e8b2d01ffa0a15736b2370b6e5064f9702c891b6").unwrap())), ..default.clone() }); // Action let json = r#" { - "receiver": { + "to": { "action": "contract_creation" } } "#; let res = serde_json::from_str::(json).unwrap(); assert_eq!(res, FilterOptions { - receiver: FilterOperator::Eq(None), + to: FilterOperator::Eq(None), ..default }); } #[test] - fn invalid_receiver_operators() { + fn invalid_to_operators() { // Multiple operators are invalid let json = r#" { - "receiver": { + "to": { "eq": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6", "action": "contract_creation" } @@ -465,7 +482,7 @@ mod tests { // Gt let json = r#" { - "receiver": { + "to": { "gt": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" } } @@ -476,7 +493,7 @@ mod tests { // Lt let json = r#" { - "receiver": { + "to": { "lt": "0xe8b2d01ffa0a15736b2370b6e5064f9702c891b6" } } @@ -487,7 +504,7 @@ mod tests { // Action (invalid value, must be "contract_creation") let json = r#" { - "receiver": { + "to": { "action": "some_invalid_value" } } @@ -498,7 +515,7 @@ mod tests { // Unknown operator let json = r#" { - "receiver": { + "to": { "abc": "0x0" } } diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 94b8b47f200..161b5234981 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1110,7 +1110,7 @@ impl miner::MinerService for Miner { .filter(|tx| { filter.as_ref().map_or(true, |filter| { let sender = tx.signed().sender(); - match filter.sender { + match filter.from { Eq(value) => sender == value, // Will always occure on `Any`, other operators // get handled during deserialization @@ -1122,7 +1122,7 @@ impl miner::MinerService for Miner { .filter(|tx| { filter.as_ref().map_or(true, |filter| { let receiver = tx.signed().receiver(); - match filter.receiver { + match filter.to { // Could apply to `Some(Address)` or `None` (for contract creation) Eq(value) => receiver == value, // Will always occure on `Any`, other operators From c85699e67e0b56c4f849b48945f068c7fb723fdb Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Wed, 26 Jun 2019 21:59:18 +0000 Subject: [PATCH 41/42] cleanup code according to suggestions, add docs --- ethcore/src/miner/filter_options.rs | 3 +++ ethcore/src/miner/miner.rs | 10 ++++---- ethcore/types/src/transaction/transaction.rs | 26 ++++++-------------- rpc/src/v1/impls/light/parity.rs | 2 +- 4 files changed, 17 insertions(+), 24 deletions(-) diff --git a/ethcore/src/miner/filter_options.rs b/ethcore/src/miner/filter_options.rs index 0b190096b3b..49afbeeb74f 100644 --- a/ethcore/src/miner/filter_options.rs +++ b/ethcore/src/miner/filter_options.rs @@ -58,6 +58,9 @@ enum Wrapper { CC, // Contract Creation } +/// Available operators for filtering options. +/// The `from` filter only accepts Any and Eq(Address) +/// The `to` filter only accepts Any, Eq(Address) and Eq(None) for contract creation. #[derive(Debug, Clone, Eq, PartialEq)] pub enum FilterOperator { Any, diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 161b5234981..95db4153781 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1121,7 +1121,7 @@ impl miner::MinerService for Miner { // Filter by receiver .filter(|tx| { filter.as_ref().map_or(true, |filter| { - let receiver = tx.signed().receiver(); + let receiver = (*tx.signed()).receiver(); match filter.to { // Could apply to `Some(Address)` or `None` (for contract creation) Eq(value) => receiver == value, @@ -1134,25 +1134,25 @@ impl miner::MinerService for Miner { // Filter by gas .filter(|tx| { filter.as_ref().map_or(true, |filter| { - match_common_filter(&filter.gas, &tx.signed().as_unsigned().gas) + 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().as_unsigned().gas_price) + 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().as_unsigned().value) + 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().as_unsigned().nonce) + match_common_filter(&filter.nonce, &(*tx.signed()).nonce) }) }) .take(max_len) diff --git a/ethcore/types/src/transaction/transaction.rs b/ethcore/types/src/transaction/transaction.rs index d031d95ab4e..e0a1f302d99 100644 --- a/ethcore/types/src/transaction/transaction.rs +++ b/ethcore/types/src/transaction/transaction.rs @@ -324,6 +324,14 @@ impl UnverifiedTransaction { self.r.is_zero() && self.s.is_zero() } + /// Returns transaction receiver, if any + pub fn receiver(&self) -> Option
{ + match self.unsigned.action { + Action::Create => None, + Action::Call(receiver) => Some(receiver), + } + } + /// Append object with a signature into RLP stream fn rlp_append_sealed_transaction(&self, s: &mut RlpStream) { s.begin_list(9); @@ -462,24 +470,6 @@ impl SignedTransaction { self.sender } - /// Returns transaction receiver, if any - pub fn receiver(&self) -> Option
{ - match self.transaction.unsigned.action { - Action::Create => None, - Action::Call(receiver) => Some(receiver), - } - } - - // Reference to the plain text transaction - pub fn as_unsigned(&self) -> &Transaction { - self.transaction.as_unsigned() - } - - /// Returns transaction hash - pub fn hash(&self) -> H256 { - self.transaction.hash - } - /// Returns a public key of the sender. pub fn public_key(&self) -> Option { self.public diff --git a/rpc/src/v1/impls/light/parity.rs b/rpc/src/v1/impls/light/parity.rs index 8849b8e58ad..06ad7b43b28 100644 --- a/rpc/src/v1/impls/light/parity.rs +++ b/rpc/src/v1/impls/light/parity.rs @@ -218,7 +218,7 @@ where .map(Into::into) } - fn pending_transactions(&self, limit: Option, _sender: Option) -> Result> { + fn pending_transactions(&self, limit: Option, _filter: Option) -> Result> { let txq = self.light_dispatch.transaction_queue.read(); let chain_info = self.light_dispatch.client.chain_info(); Ok( From 5cc8cc04cc0a194571a6241420d8ac9acc017813 Mon Sep 17 00:00:00 2001 From: lamafab <42901763+lamafab@users.noreply.github.com> Date: Thu, 27 Jun 2019 18:51:40 +0000 Subject: [PATCH 42/42] small improvements like formatting and newline --- ethcore/src/miner/filter_options.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ethcore/src/miner/filter_options.rs b/ethcore/src/miner/filter_options.rs index 49afbeeb74f..db0e67378ad 100644 --- a/ethcore/src/miner/filter_options.rs +++ b/ethcore/src/miner/filter_options.rs @@ -96,14 +96,14 @@ impl<'de, T, M> Validate<'de, T, M> for M Any | Eq(_) => Ok(val), _ => { Err(M::Error::custom( - "the from filter only supports the `eq` operator", + "the `from` filter only supports the `eq` operator", )) } } }, W::CC => { Err(M::Error::custom( - "the from filter only supports the `eq` operator", + "the `from` filter only supports the `eq` operator", )) } } @@ -119,7 +119,7 @@ impl<'de, T, M> Validate<'de, T, M> for M Eq(address) => Ok(Eq(Some(address))), _ => { Err(M::Error::custom( - "the to filter only supports the `eq` or `action` operator", + "the `to` filter only supports the `eq` or `action` operator", )) } } @@ -134,7 +134,7 @@ impl<'de, T, M> Validate<'de, T, M> for M W::O(val) => Ok(val), W::CC => { Err(M::Error::custom( - "the operator `action` is only supported by the to filter", + "the operator `action` is only supported by the `to` filter", )) } } @@ -866,4 +866,4 @@ mod tests { let res = serde_json::from_str::(json); assert!(res.is_err()); } -} \ No newline at end of file +}