Skip to content

Commit

Permalink
Add additional service transactions checking to block verifier
Browse files Browse the repository at this point in the history
  • Loading branch information
varasev committed Nov 10, 2021
1 parent 88eb7d3 commit caa2101
Show file tree
Hide file tree
Showing 14 changed files with 72 additions and 23 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions crates/concensus/miner/src/pool/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand Down
14 changes: 7 additions & 7 deletions crates/concensus/miner/src/pool/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
}

Expand Down Expand Up @@ -243,13 +243,13 @@ impl<C: Client> txpool::Verifier<Transaction>
}

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 {
Expand Down
2 changes: 1 addition & 1 deletion crates/concensus/miner/src/service_transaction_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
44 changes: 44 additions & 0 deletions crates/ethcore/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions crates/ethcore/src/executive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion crates/ethcore/src/machine/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ impl EthereumMachine {
header: &Header,
) -> Result<SignedTransaction, transaction::Error> {
// 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(),
Expand Down
2 changes: 1 addition & 1 deletion crates/ethcore/types/src/transaction/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions crates/transaction-pool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
4 changes: 2 additions & 2 deletions crates/transaction-pool/src/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/transaction-pool/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/util/version/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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 <[email protected]>"]
build = "build.rs"

Expand Down

0 comments on commit caa2101

Please sign in to comment.