From 73354d8d93ead802ac43080a7959cb2732666b51 Mon Sep 17 00:00:00 2001 From: Vladimir Komendantskiy Date: Mon, 13 Jan 2020 10:27:03 +0000 Subject: [PATCH] Set the block gas limit to the value returned by a contract call (#10928) * Block gas limit contract Lower gas limit if TxPermission.limitBlockGas. Call blockGasLimit before every block. Make the block gas limit contract a separate config option. Add `info` level logging of block gas limit switching block-gas-limit subcrate and responses to review comments simplified call_contract_before moved block_gas_limit_contract_transitions to AuRa params removed call_contract_before Update and fix test_verify_block. Remove some unused imports and functions. * Move gas limit override check to verify_block_basic. Co-authored-by: Andreas Fackler --- Cargo.lock | 16 +++++ ethcore/block-gas-limit/Cargo.toml | 20 ++++++ .../block-gas-limit/res/block_gas_limit.json | 16 +++++ ethcore/block-gas-limit/src/lib.rs | 39 ++++++++++++ ethcore/engine/src/engine.rs | 11 ++++ ethcore/engines/authority-round/Cargo.toml | 1 + ethcore/engines/authority-round/src/lib.rs | 55 +++++++++++++++- ethcore/verification/src/verification.rs | 62 +++++++++++-------- json/src/spec/authority_round.rs | 23 +++++-- 9 files changed, 212 insertions(+), 31 deletions(-) create mode 100644 ethcore/block-gas-limit/Cargo.toml create mode 100644 ethcore/block-gas-limit/res/block_gas_limit.json create mode 100644 ethcore/block-gas-limit/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index c084c344a..0c61ff5e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -177,6 +177,7 @@ dependencies = [ name = "authority-round" version = "0.1.0" dependencies = [ + "block-gas-limit 0.1.0", "block-reward 0.1.0", "client-traits 0.1.0", "common-types 0.1.0", @@ -364,6 +365,21 @@ dependencies = [ "generic-array 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "block-gas-limit" +version = "0.1.0" +dependencies = [ + "client-traits 0.1.0", + "common-types 0.1.0", + "ethabi 9.0.1 (registry+https://github.com/rust-lang/crates.io-index)", + "ethabi-contract 9.0.0 (registry+https://github.com/rust-lang/crates.io-index)", + "ethabi-derive 9.0.1 (registry+https://github.com/rust-lang/crates.io-index)", + "ethcore 1.12.0", + "ethereum-types 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)", + "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", + "spec 0.1.0", +] + [[package]] name = "block-modes" version = "0.3.3" diff --git a/ethcore/block-gas-limit/Cargo.toml b/ethcore/block-gas-limit/Cargo.toml new file mode 100644 index 000000000..1e34dac7c --- /dev/null +++ b/ethcore/block-gas-limit/Cargo.toml @@ -0,0 +1,20 @@ +[package] +description = "A crate to interact with the block gas limit contract" +name = "block-gas-limit" +version = "0.1.0" +authors = ["Parity Technologies "] +edition = "2018" +license = "GPL-3.0" + +[dependencies] +client-traits = { path = "../client-traits" } +common-types = { path = "../types" } +ethabi = "9.0.1" +ethabi-derive = "9.0.1" +ethabi-contract = "9.0.0" +ethereum-types = "0.8.0" +log = "0.4" + +[dev-dependencies] +ethcore = { path = "..", features = ["test-helpers"] } +spec = { path = "../spec" } diff --git a/ethcore/block-gas-limit/res/block_gas_limit.json b/ethcore/block-gas-limit/res/block_gas_limit.json new file mode 100644 index 000000000..bb2671b45 --- /dev/null +++ b/ethcore/block-gas-limit/res/block_gas_limit.json @@ -0,0 +1,16 @@ +[ + { + "constant": true, + "inputs": [], + "name": "blockGasLimit", + "outputs": [ + { + "name": "", + "type": "uint256" + } + ], + "payable": false, + "stateMutability": "view", + "type": "function" + } +] diff --git a/ethcore/block-gas-limit/src/lib.rs b/ethcore/block-gas-limit/src/lib.rs new file mode 100644 index 000000000..2fab781b4 --- /dev/null +++ b/ethcore/block-gas-limit/src/lib.rs @@ -0,0 +1,39 @@ +// 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 . + +//! A client interface for interacting with the block gas limit contract. + +use client_traits::BlockChainClient; +use common_types::{header::Header, ids::BlockId}; +use ethabi::FunctionOutputDecoder; +use ethabi_contract::use_contract; +use ethereum_types::{Address, U256}; +use log::{debug, error}; + +use_contract!(contract, "res/block_gas_limit.json"); + +pub fn block_gas_limit(full_client: &dyn BlockChainClient, header: &Header, address: Address) -> Option { + let (data, decoder) = contract::functions::block_gas_limit::call(); + let value = full_client.call_contract(BlockId::Hash(*header.parent_hash()), address, data).map_err(|err| { + error!(target: "block_gas_limit", "Contract call failed. Not changing the block gas limit. {:?}", err); + }).ok()?; + if value.is_empty() { + debug!(target: "block_gas_limit", "Contract call returned nothing. Not changing the block gas limit."); + None + } else { + decoder.decode(&value).ok() + } +} diff --git a/ethcore/engine/src/engine.rs b/ethcore/engine/src/engine.rs index 8ec7bba31..e31e4b857 100644 --- a/ethcore/engine/src/engine.rs +++ b/ethcore/engine/src/engine.rs @@ -347,6 +347,12 @@ pub trait Engine: Sync + Send { Ok(*header.author()) } + /// Overrides the block gas limit. Whenever this returns `Some` for a header, the next block's gas limit must be + /// exactly that value. + fn gas_limit_override(&self, _header: &Header) -> Option { + None + } + /// Get the general parameters of the chain. fn params(&self) -> &CommonParams; @@ -397,6 +403,11 @@ pub trait Engine: Sync + Send { fn decode_transaction(&self, transaction: &[u8]) -> Result { self.machine().decode_transaction(transaction) } + + /// The configured minimum gas limit. + fn min_gas_limit(&self) -> U256 { + self.params().min_gas_limit + } } /// Verifier for all blocks within an epoch with self-contained state. diff --git a/ethcore/engines/authority-round/Cargo.toml b/ethcore/engines/authority-round/Cargo.toml index 9e3011eb6..a1eaae1bc 100644 --- a/ethcore/engines/authority-round/Cargo.toml +++ b/ethcore/engines/authority-round/Cargo.toml @@ -7,6 +7,7 @@ edition = "2018" license = "GPL-3.0" [dependencies] +block-gas-limit = { path = "../../block-gas-limit" } block-reward = { path = "../../block-reward" } client-traits = { path = "../../client-traits" } common-types = { path = "../../types" } diff --git a/ethcore/engines/authority-round/src/lib.rs b/ethcore/engines/authority-round/src/lib.rs index 2a9ee7eea..3d0a82f32 100644 --- a/ethcore/engines/authority-round/src/lib.rs +++ b/ethcore/engines/authority-round/src/lib.rs @@ -42,6 +42,7 @@ use std::u64; use client_traits::{EngineClient, ForceUpdateSealing, TransactionRequest}; use engine::{Engine, ConstructedVerifier}; +use block_gas_limit::block_gas_limit; use block_reward::{self, BlockRewardContract, RewardKind}; use ethjson; use machine::{ @@ -51,6 +52,7 @@ use machine::{ use macros::map; use keccak_hash::keccak; use log::{info, debug, error, trace, warn}; +use lru_cache::LruCache; use engine::signer::EngineSigner; use parity_crypto::publickey::Signature; use io::{IoContext, IoHandler, TimerToken, IoService}; @@ -78,7 +80,6 @@ use common_types::{ transaction::SignedTransaction, }; use unexpected::{Mismatch, OutOfBounds}; - use validator_set::{ValidatorSet, SimpleList, new_validator_set}; mod finality; @@ -124,10 +125,16 @@ pub struct AuthorityRoundParams { pub strict_empty_steps_transition: u64, /// If set, enables random number contract integration. It maps the transition block to the contract address. pub randomness_contract_address: BTreeMap, + /// The addresses of contracts that determine the block gas limit with their associated block + /// numbers. + pub block_gas_limit_contract_transitions: BTreeMap, } const U16_MAX: usize = ::std::u16::MAX as usize; +/// The number of recent block hashes for which the gas limit override is memoized. +const GAS_LIMIT_OVERRIDE_CACHE_CAPACITY: usize = 10; + impl From for AuthorityRoundParams { fn from(p: ethjson::spec::AuthorityRoundParams) -> Self { let map_step_duration = |u: ethjson::uint::Uint| { @@ -180,6 +187,12 @@ impl From for AuthorityRoundParams { (block.as_u64(), addr.into()) }).collect() }); + let block_gas_limit_contract_transitions: BTreeMap<_, _> = + p.block_gas_limit_contract_transitions + .unwrap_or_default() + .into_iter() + .map(|(block_num, address)| (block_num.into(), address.into())) + .collect(); AuthorityRoundParams { step_durations, validators: new_validator_set(p.validators), @@ -196,6 +209,7 @@ impl From for AuthorityRoundParams { two_thirds_majority_transition: p.two_thirds_majority_transition.map_or_else(BlockNumber::max_value, Into::into), strict_empty_steps_transition: p.strict_empty_steps_transition.map_or(0, Into::into), randomness_contract_address, + block_gas_limit_contract_transitions, } } } @@ -565,6 +579,10 @@ pub struct AuthorityRound { received_step_hashes: RwLock>, /// If set, enables random number contract integration. It maps the transition block to the contract address. randomness_contract_address: BTreeMap, + /// The addresses of contracts that determine the block gas limit. + block_gas_limit_contract_transitions: BTreeMap, + /// Memoized gas limit overrides, by block hash. + gas_limit_override_cache: Mutex>>, } // header-chain validator. @@ -867,6 +885,8 @@ impl AuthorityRound { machine, received_step_hashes: RwLock::new(Default::default()), randomness_contract_address: our_params.randomness_contract_address, + block_gas_limit_contract_transitions: our_params.block_gas_limit_contract_transitions, + gas_limit_override_cache: Mutex::new(LruCache::new(GAS_LIMIT_OVERRIDE_CACHE_CAPACITY)), }); // Do not initialize timeouts for tests. @@ -1218,6 +1238,14 @@ impl Engine for AuthorityRound { let score = calculate_score(parent_step, current_step, current_empty_steps_len); header.set_difficulty(score); + if let Some(gas_limit) = self.gas_limit_override(header) { + trace!(target: "engine", "Setting gas limit to {} for block {}.", gas_limit, header.number()); + let parent_gas_limit = *parent.gas_limit(); + header.set_gas_limit(gas_limit); + if parent_gas_limit != gas_limit { + info!(target: "engine", "Block gas limit was changed from {} to {}.", parent_gas_limit, gas_limit); + } + } } fn sealing_state(&self) -> SealingState { @@ -1834,6 +1862,30 @@ impl Engine for AuthorityRound { fn params(&self) -> &CommonParams { self.machine.params() } + + fn gas_limit_override(&self, header: &Header) -> Option { + let (_, &address) = self.block_gas_limit_contract_transitions.range(..=header.number()).last()?; + let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) { + Some(client) => client, + None => { + error!(target: "engine", "Unable to prepare block: missing client ref."); + return None; + } + }; + let full_client = match client.as_full_client() { + Some(full_client) => full_client, + None => { + error!(target: "engine", "Failed to upgrade to BlockchainClient."); + return None; + } + }; + if let Some(limit) = self.gas_limit_override_cache.lock().get_mut(&header.hash()) { + return *limit; + } + let limit = block_gas_limit(full_client, header, address); + self.gas_limit_override_cache.lock().insert(header.hash(), limit); + limit + } } /// A helper accumulator function mapping a step duration and a step duration transition timestamp @@ -1910,6 +1962,7 @@ mod tests { strict_empty_steps_transition: 0, two_thirds_majority_transition: 0, randomness_contract_address: BTreeMap::new(), + block_gas_limit_contract_transitions: BTreeMap::new(), }; // mutate aura params diff --git a/ethcore/verification/src/verification.rs b/ethcore/verification/src/verification.rs index aa34b8e84..06a1a7d47 100644 --- a/ethcore/verification/src/verification.rs +++ b/ethcore/verification/src/verification.rs @@ -61,6 +61,14 @@ pub fn verify_block_basic(block: &Unverified, engine: &dyn Engine, check_seal: b } } + if let Some(gas_limit) = engine.gas_limit_override(&block.header) { + if *block.header.gas_limit() != gas_limit { + return Err(From::from(BlockError::InvalidGasLimit( + OutOfBounds { min: Some(gas_limit), max: Some(gas_limit), found: *block.header.gas_limit() } + ))); + } + } + for t in &block.transactions { engine.verify_transaction_basic(t, &block.header)?; } @@ -284,22 +292,24 @@ pub(crate) fn verify_header_params(header: &Header, engine: &dyn Engine, check_s found: *header.gas_used() }))); } - let min_gas_limit = engine.params().min_gas_limit; - if header.gas_limit() < &min_gas_limit { - return Err(From::from(BlockError::InvalidGasLimit(OutOfBounds { - min: Some(min_gas_limit), - max: None, - found: *header.gas_limit() - }))); - } - if let Some(limit) = engine.maximum_gas_limit() { - if header.gas_limit() > &limit { + if engine.gas_limit_override(header).is_none() { + let min_gas_limit = engine.min_gas_limit(); + if header.gas_limit() < &min_gas_limit { return Err(From::from(BlockError::InvalidGasLimit(OutOfBounds { - min: None, - max: Some(limit), + min: Some(min_gas_limit), + max: None, found: *header.gas_limit() }))); } + if let Some(limit) = engine.maximum_gas_limit() { + if header.gas_limit() > &limit { + return Err(From::from(BlockError::InvalidGasLimit(OutOfBounds { + min: None, + max: Some(limit), + found: *header.gas_limit() + }))); + } + } } let maximum_extra_data_size = engine.maximum_extra_data_size(); if header.number() != 0 && header.extra_data().len() > maximum_extra_data_size { @@ -358,8 +368,6 @@ fn verify_parent(header: &Header, parent: &Header, engine: &dyn Engine) -> Resul assert!(header.parent_hash().is_zero() || &parent.hash() == header.parent_hash(), "Parent hash should already have been verified; qed"); - let gas_limit_divisor = engine.params().gas_limit_bound_divisor; - if !engine.is_timestamp_valid(header.timestamp(), parent.timestamp()) { let now = SystemTime::now(); let min = CheckedSystemTime::checked_add(now, Duration::from_secs(parent.timestamp().saturating_add(1))) @@ -382,16 +390,18 @@ fn verify_parent(header: &Header, parent: &Header, engine: &dyn Engine) -> Resul found: header.number() }).into()); } - - let parent_gas_limit = *parent.gas_limit(); - let min_gas = parent_gas_limit - parent_gas_limit / gas_limit_divisor; - let max_gas = parent_gas_limit + parent_gas_limit / gas_limit_divisor; - if header.gas_limit() <= &min_gas || header.gas_limit() >= &max_gas { - return Err(From::from(BlockError::InvalidGasLimit(OutOfBounds { - min: Some(min_gas), - max: Some(max_gas), - found: *header.gas_limit() - }))); + if engine.gas_limit_override(header).is_none() { + let gas_limit_divisor = engine.params().gas_limit_bound_divisor; + let parent_gas_limit = *parent.gas_limit(); + let min_gas = parent_gas_limit - parent_gas_limit / gas_limit_divisor; + let max_gas = parent_gas_limit + parent_gas_limit / gas_limit_divisor; + if header.gas_limit() <= &min_gas || header.gas_limit() >= &max_gas { + return Err(From::from(BlockError::InvalidGasLimit(OutOfBounds { + min: Some(min_gas), + max: Some(max_gas), + found: *header.gas_limit() + }))); + } } Ok(()) @@ -522,7 +532,7 @@ mod tests { // that's an invalid transaction list rlp let invalid_transactions = vec![vec![0u8]]; header.set_transactions_root(ordered_trie_root(&invalid_transactions)); - header.set_gas_limit(engine.params().min_gas_limit); + header.set_gas_limit(engine.min_gas_limit()); rlp.append(&header); rlp.append_list::, _>(&invalid_transactions); rlp.append_raw(&rlp::EMPTY_LIST_RLP, 1); @@ -541,7 +551,7 @@ mod tests { let spec = spec::new_test(); let engine = &*spec.engine; - let min_gas_limit = engine.params().min_gas_limit; + let min_gas_limit = engine.min_gas_limit(); good.set_gas_limit(min_gas_limit); good.set_timestamp(40); good.set_number(10); diff --git a/json/src/spec/authority_round.rs b/json/src/spec/authority_round.rs index 69780009d..c7464b12b 100644 --- a/json/src/spec/authority_round.rs +++ b/json/src/spec/authority_round.rs @@ -97,23 +97,29 @@ pub struct AuthorityRoundParams { pub two_thirds_majority_transition: Option, /// The random number contract's address, or a map of contract transitions. pub randomness_contract_address: Option>, + /// The addresses of contracts that determine the block gas limit starting from the block number + /// associated with each of those contracts. + pub block_gas_limit_contract_transitions: Option>, } /// Authority engine deserialization. #[derive(Debug, PartialEq, Deserialize)] #[serde(deny_unknown_fields)] pub struct AuthorityRound { - /// Ethash params. + /// Authority Round parameters. pub params: AuthorityRoundParams, } #[cfg(test)] mod tests { - use super::{Address, Uint, StepDuration}; - use ethereum_types::{U256, H160}; - use crate::spec::{validator_set::ValidatorSet, authority_round::AuthorityRound}; use std::str::FromStr; + use ethereum_types::{U256, H160}; + use serde_json; + + use super::{Address, Uint, StepDuration}; + use crate::{spec::{validator_set::ValidatorSet, authority_round::AuthorityRound}}; + #[test] fn authority_round_deserialization() { let s = r#"{ @@ -130,6 +136,10 @@ mod tests { "randomnessContractAddress": { "10": "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "20": "0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + }, + "blockGasLimitContractTransitions": { + "10": "0x1000000000000000000000000000000000000001", + "20": "0x2000000000000000000000000000000000000002" } } }"#; @@ -149,5 +159,10 @@ mod tests { (Uint(10.into()), Address(H160::from_str("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").unwrap())), (Uint(20.into()), Address(H160::from_str("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb").unwrap())), ].into_iter().collect()); + let expected_bglc = + [(Uint(10.into()), Address(H160::from_str("1000000000000000000000000000000000000001").unwrap())), + (Uint(20.into()), Address(H160::from_str("2000000000000000000000000000000000000002").unwrap()))]; + assert_eq!(deserialized.params.block_gas_limit_contract_transitions, + Some(expected_bglc.to_vec().into_iter().collect())); } }