From caa210107eaa1d935d6bc7249412735f34e1a628 Mon Sep 17 00:00:00 2001 From: POA <33550681+poa@users.noreply.github.com> Date: Wed, 10 Nov 2021 16:36:17 +0300 Subject: [PATCH] Add additional service transactions checking to block verifier --- CHANGELOG.md | 5 +++ Cargo.lock | 4 +- Cargo.toml | 2 +- crates/concensus/miner/src/pool/mod.rs | 4 +- crates/concensus/miner/src/pool/verifier.rs | 14 +++--- .../miner/src/service_transaction_checker.rs | 2 +- crates/ethcore/src/client/client.rs | 44 +++++++++++++++++++ crates/ethcore/src/executive.rs | 4 +- crates/ethcore/src/machine/impls.rs | 2 +- .../types/src/transaction/transaction.rs | 2 +- crates/transaction-pool/src/lib.rs | 4 +- crates/transaction-pool/src/pool.rs | 4 +- crates/transaction-pool/src/tests/mod.rs | 2 +- crates/util/version/Cargo.toml | 2 +- 14 files changed, 72 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd3736c6d..adcffe514 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## OpenEthereum v3.3.0-rc.16 + +Enhancements: +* Additionally check zero gas price transactions by block verifier + ## OpenEthereum v3.3.0-rc.15 * Revert eip1559BaseFeeMinValue activation on xDai at London hardfork block diff --git a/Cargo.lock b/Cargo.lock index 00fae65fb..288f84c3a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2932,7 +2932,7 @@ checksum = "2839e79665f131bdb5782e51f2c6c9599c133c6098982a54c794358bf432529c" [[package]] name = "openethereum" -version = "3.3.0-rc.15" +version = "3.3.0-rc.16" dependencies = [ "ansi_term 0.10.2", "atty", @@ -3282,7 +3282,7 @@ dependencies = [ [[package]] name = "parity-version" -version = "3.3.0-rc.15" +version = "3.3.0-rc.16" dependencies = [ "parity-bytes", "rlp", diff --git a/Cargo.toml b/Cargo.toml index 9427865b1..3839639a9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ description = "OpenEthereum" name = "openethereum" # NOTE Make sure to update util/version/Cargo.toml as well -version = "3.3.0-rc.15" +version = "3.3.0-rc.16" license = "GPL-3.0" authors = [ "OpenEthereum developers", diff --git a/crates/concensus/miner/src/pool/mod.rs b/crates/concensus/miner/src/pool/mod.rs index 12bfdfa4e..9201cae3b 100644 --- a/crates/concensus/miner/src/pool/mod.rs +++ b/crates/concensus/miner/src/pool/mod.rs @@ -189,8 +189,8 @@ impl txpool::VerifiedTransaction for VerifiedTransaction { &self.sender } - fn is_service(&self) -> bool { - self.transaction.is_service() + fn has_zero_gas_price(&self) -> bool { + self.transaction.has_zero_gas_price() } } diff --git a/crates/concensus/miner/src/pool/verifier.rs b/crates/concensus/miner/src/pool/verifier.rs index 55f13cc86..f544ed369 100644 --- a/crates/concensus/miner/src/pool/verifier.rs +++ b/crates/concensus/miner/src/pool/verifier.rs @@ -131,12 +131,12 @@ impl Transaction { } } - /// Cheeck if transaction is service transaction - pub fn is_service(&self) -> bool { + /// Check if transaction has zero gas price + pub fn has_zero_gas_price(&self) -> bool { match *self { - Transaction::Unverified(ref tx) => tx.is_service(), - Transaction::Retracted(ref tx) => tx.is_service(), - Transaction::Local(ref tx) => tx.is_service(), + Transaction::Unverified(ref tx) => tx.has_zero_gas_price(), + Transaction::Retracted(ref tx) => tx.has_zero_gas_price(), + Transaction::Local(ref tx) => tx.has_zero_gas_price(), } } @@ -243,13 +243,13 @@ impl txpool::Verifier } let is_own = tx.is_local(); - let is_service = tx.is_service(); + let has_zero_gas_price = tx.has_zero_gas_price(); // Quick exit for non-service and non-local transactions // // We're checking if the transaction is below configured minimal gas price // or the effective minimal gas price in case the pool is full. - if !is_service && !is_own { + if !has_zero_gas_price && !is_own { let effective_priority_fee = tx.effective_priority_fee(self.options.block_base_fee); if effective_priority_fee < self.options.minimal_gas_price { diff --git a/crates/concensus/miner/src/service_transaction_checker.rs b/crates/concensus/miner/src/service_transaction_checker.rs index f426571fa..0ad74db88 100644 --- a/crates/concensus/miner/src/service_transaction_checker.rs +++ b/crates/concensus/miner/src/service_transaction_checker.rs @@ -72,7 +72,7 @@ impl ServiceTransactionChecker { SERVICE_TRANSACTION_CONTRACT_REGISTRY_NAME.to_owned(), BlockId::Latest, ) - .ok_or_else(|| "contract is not configured")?; + .ok_or_else(|| "Certifier contract is not configured")?; self.call_contract(client, contract_address, sender) .and_then(|allowed| { if let Some(mut cache) = self.certified_addresses_cache.try_write() { diff --git a/crates/ethcore/src/client/client.rs b/crates/ethcore/src/client/client.rs index 8a4110a8a..739d1696a 100644 --- a/crates/ethcore/src/client/client.rs +++ b/crates/ethcore/src/client/client.rs @@ -488,6 +488,50 @@ impl Importer { .epoch_transition(parent.number(), *header.parent_hash()) .is_some(); + // Check if zero gas price transactions are certified to be service transactions + // using the Certifier contract. If they are not certified, the block is treated as invalid. + let service_transaction_checker = self.miner.service_transaction_checker(); + if service_transaction_checker.is_some() { + match service_transaction_checker.unwrap().refresh_cache(client) { + Ok(true) => { + trace!(target: "client", "Service transaction cache was refreshed successfully"); + } + Ok(false) => { + trace!(target: "client", "Registrar or/and service transactions contract does not exist"); + } + Err(e) => { + error!(target: "client", "Error occurred while refreshing service transaction cache: {}", e) + } + }; + }; + for t in &block.transactions { + if t.has_zero_gas_price() { + match self.miner.service_transaction_checker() { + None => { + let e = "Service transactions are not allowed. You need to enable Certifier contract."; + warn!(target: "client", "Service tx checker error: {:?}", e); + bail!(e); + } + Some(ref checker) => match checker.check(client, &t) { + Ok(true) => {} + Ok(false) => { + let e = format!( + "Service transactions are not allowed for the sender {:?}", + t.sender() + ); + warn!(target: "client", "Service tx checker error: {:?}", e); + bail!(e); + } + Err(e) => { + debug!(target: "client", "Unable to verify service transaction: {:?}", e); + warn!(target: "client", "Service tx checker error: {:?}", e); + bail!(e); + } + }, + } + }; + } + // t_nb 8.0 Block enacting. Execution of transactions. let enact_result = enact_verified( block, diff --git a/crates/ethcore/src/executive.rs b/crates/ethcore/src/executive.rs index a91da17a2..b918ec05f 100644 --- a/crates/ethcore/src/executive.rs +++ b/crates/ethcore/src/executive.rs @@ -1207,7 +1207,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { } // ensure that the user was willing to at least pay the base fee - if t.tx().gas_price < self.info.base_fee.unwrap_or_default() && !t.is_service() { + if t.tx().gas_price < self.info.base_fee.unwrap_or_default() && !t.has_zero_gas_price() { return Err(ExecutionError::GasPriceLowerThanBaseFee { gas_price: t.tx().gas_price, base_fee: self.info.base_fee.unwrap_or_default(), @@ -1516,7 +1516,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { // Up until now, fees_value is calculated for each type of transaction based on their gas prices // Now, if eip1559 is activated, burn the base fee // miner only receives the inclusion fee; note that the base fee is not given to anyone (it is burned) - let burnt_fee = if schedule.eip1559 && !t.is_service() { + let burnt_fee = if schedule.eip1559 && !t.has_zero_gas_price() { let (fee, overflow_3) = gas_used.overflowing_mul(self.info.base_fee.unwrap_or_default()); if overflow_3 { diff --git a/crates/ethcore/src/machine/impls.rs b/crates/ethcore/src/machine/impls.rs index 283e1c326..a17d94cdc 100644 --- a/crates/ethcore/src/machine/impls.rs +++ b/crates/ethcore/src/machine/impls.rs @@ -376,7 +376,7 @@ impl EthereumMachine { header: &Header, ) -> Result { // ensure that the user was willing to at least pay the base fee - if t.tx().gas_price < header.base_fee().unwrap_or_default() && !t.is_service() { + if t.tx().gas_price < header.base_fee().unwrap_or_default() && !t.has_zero_gas_price() { return Err(transaction::Error::GasPriceLowerThanBaseFee { gas_price: t.tx().gas_price, base_fee: header.base_fee().unwrap_or_default(), diff --git a/crates/ethcore/types/src/transaction/transaction.rs b/crates/ethcore/types/src/transaction/transaction.rs index 1954e665d..7f6d1a234 100644 --- a/crates/ethcore/types/src/transaction/transaction.rs +++ b/crates/ethcore/types/src/transaction/transaction.rs @@ -657,7 +657,7 @@ impl TypedTransaction { .unwrap_or_default() } - pub fn is_service(&self) -> bool { + pub fn has_zero_gas_price(&self) -> bool { match self { Self::EIP1559Transaction(tx) => { tx.tx().gas_price.is_zero() && tx.max_priority_fee_per_gas.is_zero() diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index 20541da1d..0cb0ae6fa 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -114,6 +114,6 @@ pub trait VerifiedTransaction: fmt::Debug { /// Transaction sender fn sender(&self) -> &Self::Sender; - /// Is it a service transaction? - fn is_service(&self) -> bool; + /// Does it have zero gas price? + fn has_zero_gas_price(&self) -> bool; } diff --git a/crates/transaction-pool/src/pool.rs b/crates/transaction-pool/src/pool.rs index bf071345c..a33c15bde 100644 --- a/crates/transaction-pool/src/pool.rs +++ b/crates/transaction-pool/src/pool.rs @@ -651,7 +651,7 @@ where Readiness::Ready => { //return transaction with score higher or equal to desired if score >= &self.includable_boundary - || tx.transaction.is_service() + || tx.transaction.has_zero_gas_price() { return Some(tx.transaction.clone()); } @@ -740,7 +740,7 @@ where if tx_state == Readiness::Ready { //return transaction with score higher or equal to desired if best.score >= self.includable_boundary - || best.transaction.transaction.is_service() + || best.transaction.transaction.has_zero_gas_price() { return Some(best.transaction.transaction); } diff --git a/crates/transaction-pool/src/tests/mod.rs b/crates/transaction-pool/src/tests/mod.rs index 81f4ea95c..3e1d87c6a 100644 --- a/crates/transaction-pool/src/tests/mod.rs +++ b/crates/transaction-pool/src/tests/mod.rs @@ -50,7 +50,7 @@ impl VerifiedTransaction for Transaction { fn sender(&self) -> &Address { &self.sender } - fn is_service(&self) -> bool { + fn has_zero_gas_price(&self) -> bool { false } } diff --git a/crates/util/version/Cargo.toml b/crates/util/version/Cargo.toml index 8b126f188..e3550800a 100644 --- a/crates/util/version/Cargo.toml +++ b/crates/util/version/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "parity-version" # NOTE: this value is used for OpenEthereum version string (via env CARGO_PKG_VERSION) -version = "3.3.0-rc.15" +version = "3.3.0-rc.16" authors = ["Parity Technologies "] build = "build.rs"