From 24378f3c44043852b5bd1d418c2311420d6fb43c Mon Sep 17 00:00:00 2001 From: Vlad Lupashevskyi Date: Thu, 21 Dec 2017 22:42:36 +0200 Subject: [PATCH 1/5] Added checking tx-type using transactions permission contract for miners --- ethcore/src/client/client.rs | 2 ++ ethcore/src/client/test_client.rs | 2 ++ ethcore/src/client/traits.rs | 3 +++ ethcore/src/miner/miner.rs | 20 +++++++++++++++++++- ethcore/src/miner/transaction_queue.rs | 5 +++++ 5 files changed, 31 insertions(+), 1 deletion(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 6b5dbe59c..4ba470796 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1840,6 +1840,8 @@ impl BlockChainClient for Client { } impl MiningBlockChainClient for Client { + fn as_block_chain_client(&self) -> &BlockChainClient { self } + fn latest_schedule(&self) -> Schedule { self.engine.schedule(self.latest_env_info().number) } diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index d06f94768..cc2f08bab 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -365,6 +365,8 @@ pub fn get_temp_state_db() -> GuardedTempResult { } impl MiningBlockChainClient for TestBlockChainClient { + fn as_block_chain_client(&self) -> &BlockChainClient { self } + fn latest_schedule(&self) -> Schedule { Schedule::new_post_eip150(24576, true, true, true) } diff --git a/ethcore/src/client/traits.rs b/ethcore/src/client/traits.rs index 3ff60baf9..58a05e4e0 100644 --- a/ethcore/src/client/traits.rs +++ b/ethcore/src/client/traits.rs @@ -308,6 +308,9 @@ pub trait MiningBlockChainClient: BlockChainClient { /// Returns latest schedule. fn latest_schedule(&self) -> Schedule; + + /// Returns base of this trait + fn as_block_chain_client(&self) -> &BlockChainClient; } /// Client facilities used by internally sealing Engines. diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 4f1d5b687..389531683 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -411,6 +411,7 @@ impl Miner { }; let mut invalid_transactions = HashSet::new(); + let mut non_allowed_transactions = HashSet::new(); let mut transactions_to_penalize = HashSet::new(); let block_number = open_block.block().fields().header.number(); @@ -419,7 +420,15 @@ impl Miner { for tx in transactions { let hash = tx.hash(); let start = Instant::now(); - let result = open_block.push_transaction(tx, None); + // Check whether transaction type is allowed for sender + let result = match self.engine.machine().verify_transaction(&tx, open_block.header(), chain.as_block_chain_client()) { + Err(Error::Transaction(TransactionError::NotAllowed)) => { + Err(From::from(TransactionError::NotAllowed)) + } + _ => { + open_block.push_transaction(tx, None) + } + }; let took = start.elapsed(); // Check for heavy transactions @@ -460,6 +469,12 @@ impl Miner { }, // already have transaction - ignore Err(Error::Transaction(TransactionError::AlreadyImported)) => {}, + Err(Error::Transaction(TransactionError::NotAllowed)) => { + non_allowed_transactions.insert(hash); + debug!(target: "miner", + "Skipping non-allowed transaction for sender {:?}", + hash); + }, Err(e) => { invalid_transactions.insert(hash); debug!(target: "miner", @@ -482,6 +497,9 @@ impl Miner { for hash in invalid_transactions { queue.remove(&hash, &fetch_nonce, RemovalReason::Invalid); } + for hash in non_allowed_transactions { + queue.remove(&hash, &fetch_nonce, RemovalReason::NotAllowed); + } for hash in transactions_to_penalize { queue.penalize(&hash); } diff --git a/ethcore/src/miner/transaction_queue.rs b/ethcore/src/miner/transaction_queue.rs index 9cb835b28..dbd20615b 100644 --- a/ethcore/src/miner/transaction_queue.rs +++ b/ethcore/src/miner/transaction_queue.rs @@ -545,6 +545,8 @@ pub enum RemovalReason { Invalid, /// Transaction was canceled Canceled, + /// Transaction is not allowed, + NotAllowed, } /// Point in time when transaction was inserted. @@ -1014,6 +1016,9 @@ impl TransactionQueue { RemovalReason::Invalid => self.local_transactions.mark_invalid( transaction.transaction.into() ), + RemovalReason::NotAllowed => self.local_transactions.mark_invalid( + transaction.transaction.into() + ), RemovalReason::Canceled => self.local_transactions.mark_canceled( PendingTransaction::new(transaction.transaction, transaction.condition) ), From 9d35cc188116a2606a27cb92aad3c76bb3ad16c9 Mon Sep 17 00:00:00 2001 From: Vlad Lupashevskyi Date: Fri, 22 Dec 2017 22:48:37 +0200 Subject: [PATCH 2/5] "from" replaced with ".into()" --- 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 389531683..9868edbfd 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -423,7 +423,7 @@ impl Miner { // Check whether transaction type is allowed for sender let result = match self.engine.machine().verify_transaction(&tx, open_block.header(), chain.as_block_chain_client()) { Err(Error::Transaction(TransactionError::NotAllowed)) => { - Err(From::from(TransactionError::NotAllowed)) + Err(TransactionError::NotAllowed.into()) } _ => { open_block.push_transaction(tx, None) From d6ae6e315e6cc42ea1151ed48a33460a2b72e5fe Mon Sep 17 00:00:00 2001 From: Vlad Lupashevskyi Date: Fri, 22 Dec 2017 22:50:09 +0200 Subject: [PATCH 3/5] Check tx-type before importing transactions to the queue --- ethcore/src/miner/miner.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 9868edbfd..35e00acef 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -698,6 +698,15 @@ impl Miner { Err(e) }, Ok(transaction) => { + // This check goes here because verify_transaction takes SignedTransaction parameter + match self.engine.machine().verify_transaction(&transaction, &best_block_header, client.as_block_chain_client()) { + Err(Error::Transaction(TransactionError::NotAllowed)) => { + debug!(target: "miner", "Rejected disallowed tx {:?}", hash); + return Err(Error::Transaction(TransactionError::NotAllowed)); + } + _ => {} + } + let origin = self.accounts.as_ref().and_then(|accounts| { match accounts.has_account(transaction.sender()).unwrap_or(false) { true => Some(TransactionOrigin::Local), From 0d5603eeceb178fe652f68bf435aeae89187064f Mon Sep 17 00:00:00 2001 From: Vlad Lupashevskyi Date: Fri, 22 Dec 2017 23:14:24 +0200 Subject: [PATCH 4/5] Use is_err instead of match --- ethcore/src/miner/miner.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 35e00acef..06c35c5e4 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -699,12 +699,9 @@ impl Miner { }, Ok(transaction) => { // This check goes here because verify_transaction takes SignedTransaction parameter - match self.engine.machine().verify_transaction(&transaction, &best_block_header, client.as_block_chain_client()) { - Err(Error::Transaction(TransactionError::NotAllowed)) => { + if self.engine.machine().verify_transaction(&transaction, &best_block_header, client.as_block_chain_client()).is_err() { debug!(target: "miner", "Rejected disallowed tx {:?}", hash); return Err(Error::Transaction(TransactionError::NotAllowed)); - } - _ => {} } let origin = self.accounts.as_ref().and_then(|accounts| { From 31ffb467f58ac01306aaab8d14212888aa7491f6 Mon Sep 17 00:00:00 2001 From: Vlad Lupashevskyi Date: Sat, 23 Dec 2017 00:00:27 +0200 Subject: [PATCH 5/5] Passing verify tx errors to the caller --- ethcore/src/miner/miner.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 06c35c5e4..a40e7b4a1 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -699,10 +699,7 @@ impl Miner { }, Ok(transaction) => { // This check goes here because verify_transaction takes SignedTransaction parameter - if self.engine.machine().verify_transaction(&transaction, &best_block_header, client.as_block_chain_client()).is_err() { - debug!(target: "miner", "Rejected disallowed tx {:?}", hash); - return Err(Error::Transaction(TransactionError::NotAllowed)); - } + self.engine.machine().verify_transaction(&transaction, &best_block_header, client.as_block_chain_client())?; let origin = self.accounts.as_ref().and_then(|accounts| { match accounts.has_account(transaction.sender()).unwrap_or(false) {