From 98e435bd598daae1bb932b18e950df2082be8e4a Mon Sep 17 00:00:00 2001 From: Andreas Fackler Date: Tue, 9 Apr 2019 15:53:35 +0200 Subject: [PATCH 1/4] Lower gas limit if TxPermission.limitBlockGas. --- ethcore/call-contract/src/call_contract.rs | 5 +++ ethcore/private-tx/src/key_server_keys.rs | 6 ++- ethcore/res/contracts/tx_acl_gas_price.json | 14 +++++++ ethcore/src/client/client.rs | 28 ++++++++++++++ ethcore/src/client/test_client.rs | 1 + ethcore/src/engines/authority_round/mod.rs | 42 ++++++++++++++++++++- ethcore/src/engines/mod.rs | 6 +++ ethcore/src/machine.rs | 5 +++ ethcore/src/tx_filter.rs | 5 +++ ethcore/src/verification/verification.rs | 42 +++++++++++++-------- 10 files changed, 137 insertions(+), 17 deletions(-) diff --git a/ethcore/call-contract/src/call_contract.rs b/ethcore/call-contract/src/call_contract.rs index 8b042f0833c..119e6c947e9 100644 --- a/ethcore/call-contract/src/call_contract.rs +++ b/ethcore/call-contract/src/call_contract.rs @@ -18,12 +18,17 @@ use bytes::Bytes; use ethereum_types::Address; +use types::header::Header; use types::ids::BlockId; /// Provides `call_contract` method pub trait CallContract { /// Like `call`, but with various defaults. Designed to be used for calling contracts. fn call_contract(&self, id: BlockId, address: Address, data: Bytes) -> Result; + + /// Makes a constant call to a contract, at the block corresponding to the given header. Fails if the block is not + /// in the database. + fn call_contract_at(&self, header: &Header, address: Address, data: Bytes) -> Result; } /// Provides information on a blockchain service and it's registry diff --git a/ethcore/private-tx/src/key_server_keys.rs b/ethcore/private-tx/src/key_server_keys.rs index 28d9b3cb91d..09bcb882bcd 100644 --- a/ethcore/private-tx/src/key_server_keys.rs +++ b/ethcore/private-tx/src/key_server_keys.rs @@ -160,6 +160,10 @@ mod tests { impl CallContract for DummyRegistryClient { fn call_contract(&self, _id: BlockId, _address: Address, _data: Bytes) -> Result { Ok(vec![]) } + + fn call_contract_at(&self, header: &Header, address: Address, data: Bytes) -> Result { + Ok(vec![]) + } } #[test] @@ -170,4 +174,4 @@ mod tests { keys_data.update_acl_contract(); assert_eq!(keys_data.keys_acl_contract.read().unwrap(), key.address()); } -} \ No newline at end of file +} diff --git a/ethcore/res/contracts/tx_acl_gas_price.json b/ethcore/res/contracts/tx_acl_gas_price.json index bc355dee7e6..e4edfe8791a 100644 --- a/ethcore/res/contracts/tx_acl_gas_price.json +++ b/ethcore/res/contracts/tx_acl_gas_price.json @@ -41,6 +41,20 @@ "stateMutability": "view", "type": "function" }, + { + "constant": true, + "inputs": [], + "name": "limitBlockGas", + "outputs": [ + { + "name": "", + "type": "bool" + } + ], + "payable": false, + "stateMutability": "view", + "type": "function" + }, { "constant": true, "inputs": [ diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index e45edf3fad0..6d7e307e9b9 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1456,6 +1456,34 @@ impl CallContract for Client { .map_err(|e| format!("{:?}", e)) .map(|executed| executed.output) } + + fn call_contract_at(&self, header: &Header, address: Address, data: Bytes) -> Result { + let db = self.state_db.read().boxed_clone(); + + // early exit for pruned blocks + if db.is_pruned() && self.pruning_info().earliest_state > header.number() { + return Err(CallError::StatePruned.to_string()); + } + + let root = header.state_root(); + let nonce = self.engine.account_start_nonce(header.number()); + let mut state = State::from_existing(db, *root, nonce, self.factories.clone()) + .map_err(|_| CallError::StatePruned.to_string())?; + + let from = Address::default(); + let transaction = transaction::Transaction { + nonce: state.nonce(&from).unwrap_or_else(|_| self.engine.account_start_nonce(0)), + action: Action::Call(address), + gas: U256::from(50_000_000), + gas_price: U256::default(), + value: U256::default(), + data: data, + }.fake_sign(from); + + self.call(&transaction, Default::default(), &mut state, header) + .map_err(|e| format!("{:?}", e)) + .map(|executed| executed.output) + } } impl ImportBlock for Client { diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index 77fd3d77606..b8f513d23d2 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -523,6 +523,7 @@ impl BlockInfo for TestBlockChainClient { impl CallContract for TestBlockChainClient { fn call_contract(&self, _id: BlockId, _address: Address, _data: Bytes) -> Result { Ok(vec![]) } + fn call_contract_at(&self, header: &Header, address: Address, data: Bytes) -> Result { Ok(vec![]) } } impl TransactionInfo for TestBlockChainClient { diff --git a/ethcore/src/engines/authority_round/mod.rs b/ethcore/src/engines/authority_round/mod.rs index 7fbdaaab027..8e8f2c72199 100644 --- a/ethcore/src/engines/authority_round/mod.rs +++ b/ethcore/src/engines/authority_round/mod.rs @@ -32,7 +32,8 @@ use engines::{Engine, Seal, SealingState, EngineError, ConstructedVerifier}; use engines::block_reward; use engines::block_reward::{BlockRewardContract, RewardKind}; use error::{Error, ErrorKind, BlockError}; -use ethjson::{spec::StepDuration}; +use ethabi::FunctionOutputDecoder; +use ethjson::{self, spec::StepDuration}; use machine::{AuxiliaryData, Call, EthereumMachine}; use hash::keccak; use super::signer::EngineSigner; @@ -44,6 +45,7 @@ use itertools::{self, Itertools}; use rlp::{encode, Decodable, DecoderError, Encodable, RlpStream, Rlp}; use ethereum_types::{H256, H520, Address, U128, U256}; use parking_lot::{Mutex, RwLock}; +use tx_filter::transact_acl_gas_price; use types::BlockNumber; use types::ancestry_action::AncestryAction; use types::header::{Header, ExtendedHeader}; @@ -1107,6 +1109,9 @@ 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(parent) { + header.set_gas_limit(gas_limit); + } } fn sealing_state(&self) -> SealingState { @@ -1783,6 +1788,41 @@ impl Engine for AuthorityRound { finalized.into_iter().map(AncestryAction::MarkFinalized).collect() } + + fn gas_limit_override(&self, parent: &Header) -> Option { + // TODO: Use target gas limit? Must exactly agree in all nodes! + let default_gas_limit = 10_000_000.into(); + let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) { + Some(client) => client, + None => { + debug!(target: "engine", "Unable to prepare block: missing client ref."); + return Some(default_gas_limit); + } + }; + let full_client = match client.as_full_client() { + Some(full_client) => full_client, + None => { + debug!(target: "engine", "Failed to upgrade to BlockchainClient."); + return Some(default_gas_limit); + } + }; + + let address = match self.machine.tx_filter() { + Some(tx_filter) => *tx_filter.contract_address(), + None => { + debug!(target: "engine", "Not transaction filter configured. Not changing the block gas limit."); + return Some(default_gas_limit); + } + }; + + let (data, decoder) = transact_acl_gas_price::functions::limit_block_gas::call(); + match full_client.call_contract_at(parent, address, data).ok().and_then(|value| decoder.decode(&value).ok()) { + Some(true) => return Some(2_000_000.into()), + Some(false) => (), + None => debug!(target: "engine", "Failed to call limitBlockGas."), + }; + Some(default_gas_limit) + } } #[cfg(test)] diff --git a/ethcore/src/engines/mod.rs b/ethcore/src/engines/mod.rs index dd41fc0e757..ab977606fe9 100644 --- a/ethcore/src/engines/mod.rs +++ b/ethcore/src/engines/mod.rs @@ -457,6 +457,12 @@ pub trait Engine: Sync + Send { /// Check whether the given new block is the best block, after finalization check. fn fork_choice(&self, new: &M::ExtendedHeader, best: &M::ExtendedHeader) -> ForkChoice; + + /// 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, parent: &Header) -> Option { + None + } } /// Check whether a given block is the best block based on the default total difficulty rule. diff --git a/ethcore/src/machine.rs b/ethcore/src/machine.rs index a14cb3bd296..46925995c0a 100644 --- a/ethcore/src/machine.rs +++ b/ethcore/src/machine.rs @@ -108,6 +108,11 @@ impl EthereumMachine { pub fn ethash_extensions(&self) -> Option<&EthashExtensions> { self.ethash_extensions.as_ref() } + + /// Get a reference to the transaction filter, if present. + pub fn tx_filter(&self) -> Option<&Arc> { + self.tx_filter.as_ref() + } } impl EthereumMachine { diff --git a/ethcore/src/tx_filter.rs b/ethcore/src/tx_filter.rs index 2d67598ee85..3ee06a74d01 100644 --- a/ethcore/src/tx_filter.rs +++ b/ethcore/src/tx_filter.rs @@ -64,6 +64,11 @@ impl TransactionFilter { ) } + /// Get a reference to the contract address. + pub fn contract_address(&self) -> &Address { + &self.contract_address + } + /// Check if transaction is allowed at given block. pub fn transaction_allowed(&self, parent_hash: &H256, block_number: BlockNumber, transaction: &SignedTransaction, client: &C) -> bool { if block_number < self.transition_block { return true; } diff --git a/ethcore/src/verification/verification.rs b/ethcore/src/verification/verification.rs index 64b51471c67..275264600a9 100644 --- a/ethcore/src/verification/verification.rs +++ b/ethcore/src/verification/verification.rs @@ -283,13 +283,18 @@ pub fn verify_header_params(header: &Header, engine: &EthEngine, is_full: bool, if header.gas_used() > header.gas_limit() { return Err(From::from(BlockError::TooMuchGasUsed(OutOfBounds { max: Some(*header.gas_limit()), min: None, 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 { - return Err(From::from(::error::BlockError::InvalidGasLimit(OutOfBounds { min: None, max: Some(limit), found: *header.gas_limit() }))); + // TODO: This should be called with the _parent_ instead, and the value should be checked. + // Alternatively, only check in `verify_parent` below, and add another function to the engine that returns `true` if + // the engine uses a gas limit override. + if engine.gas_limit_override(header).is_none() { + 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 { + return Err(From::from(::error::BlockError::InvalidGasLimit(OutOfBounds { min: None, max: Some(limit), found: *header.gas_limit() }))); + } } } let maximum_extra_data_size = engine.maximum_extra_data_size(); @@ -325,13 +330,11 @@ pub fn verify_header_params(header: &Header, engine: &EthEngine, is_full: bool, Ok(()) } -/// Check header parameters agains parent header. +/// Check header parameters against parent header. fn verify_parent(header: &Header, parent: &Header, engine: &EthEngine) -> Result<(), Error> { 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 = now.checked_add(Duration::from_secs(parent.timestamp().saturating_add(1))) @@ -348,11 +351,20 @@ fn verify_parent(header: &Header, parent: &Header, engine: &EthEngine) -> Result return Err(BlockError::RidiculousNumber(OutOfBounds { min: Some(1), max: None, 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 let Some(gas_limit) = engine.gas_limit_override(parent) { + if *header.gas_limit() != gas_limit { + return Err(From::from(BlockError::InvalidGasLimit( + OutOfBounds { min: Some(gas_limit), max: Some(gas_limit), found: *header.gas_limit() } + ))); + } + } else { + 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(()) From 9561a1712c8021cdfbd4b5a9f3f30c912851253b Mon Sep 17 00:00:00 2001 From: Andreas Fackler Date: Thu, 4 Jul 2019 11:13:51 +0200 Subject: [PATCH 2/4] Call blockGasLimit before every block. --- ethcore/call-contract/src/call_contract.rs | 7 +++-- ethcore/private-tx/src/key_server_keys.rs | 2 +- ethcore/res/contracts/tx_acl_gas_price.json | 4 +-- ethcore/src/client/client.rs | 25 ++++++++++++---- ethcore/src/client/test_client.rs | 5 +++- ethcore/src/engines/authority_round/mod.rs | 32 +++++++++++---------- ethcore/src/engines/mod.rs | 2 +- ethcore/src/verification/verification.rs | 19 ++++++------ 8 files changed, 57 insertions(+), 39 deletions(-) diff --git a/ethcore/call-contract/src/call_contract.rs b/ethcore/call-contract/src/call_contract.rs index 119e6c947e9..39c9931058c 100644 --- a/ethcore/call-contract/src/call_contract.rs +++ b/ethcore/call-contract/src/call_contract.rs @@ -26,9 +26,10 @@ pub trait CallContract { /// Like `call`, but with various defaults. Designed to be used for calling contracts. fn call_contract(&self, id: BlockId, address: Address, data: Bytes) -> Result; - /// Makes a constant call to a contract, at the block corresponding to the given header. Fails if the block is not - /// in the database. - fn call_contract_at(&self, header: &Header, address: Address, data: Bytes) -> Result; + /// Makes a constant call to a contract, at the beginning of the block corresponding to the given header, i.e. with + /// the EVM state after the block's parent, but with the new header's block number. Fails if the parent is not in + /// the database. + fn call_contract_before(&self, header: &Header, address: Address, data: Bytes) -> Result; } /// Provides information on a blockchain service and it's registry diff --git a/ethcore/private-tx/src/key_server_keys.rs b/ethcore/private-tx/src/key_server_keys.rs index 09bcb882bcd..94e1e31495a 100644 --- a/ethcore/private-tx/src/key_server_keys.rs +++ b/ethcore/private-tx/src/key_server_keys.rs @@ -161,7 +161,7 @@ mod tests { impl CallContract for DummyRegistryClient { fn call_contract(&self, _id: BlockId, _address: Address, _data: Bytes) -> Result { Ok(vec![]) } - fn call_contract_at(&self, header: &Header, address: Address, data: Bytes) -> Result { + fn call_contract_before(&self, _header: &Header, _address: Address, _data: Bytes) -> Result { Ok(vec![]) } } diff --git a/ethcore/res/contracts/tx_acl_gas_price.json b/ethcore/res/contracts/tx_acl_gas_price.json index e4edfe8791a..a8a2af22152 100644 --- a/ethcore/res/contracts/tx_acl_gas_price.json +++ b/ethcore/res/contracts/tx_acl_gas_price.json @@ -44,11 +44,11 @@ { "constant": true, "inputs": [], - "name": "limitBlockGas", + "name": "blockGasLimit", "outputs": [ { "name": "", - "type": "bool" + "type": "uint256" } ], "payable": false, diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 6d7e307e9b9..f21806f9df7 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1457,7 +1457,7 @@ impl CallContract for Client { .map(|executed| executed.output) } - fn call_contract_at(&self, header: &Header, address: Address, data: Bytes) -> Result { + fn call_contract_before(&self, header: &Header, address: Address, data: Bytes) -> Result { let db = self.state_db.read().boxed_clone(); // early exit for pruned blocks @@ -1465,9 +1465,13 @@ impl CallContract for Client { return Err(CallError::StatePruned.to_string()); } - let root = header.state_root(); + let parent = self.chain.read().block_header_data(header.parent_hash()).ok_or_else(|| { + error!(target: "client", "Failed to call contract at {:?}'s child", header.parent_hash()); + format!("Failed to call contract at {:?}'s child", header.parent_hash()) + })?; + let root = parent.state_root(); let nonce = self.engine.account_start_nonce(header.number()); - let mut state = State::from_existing(db, *root, nonce, self.factories.clone()) + let mut state = State::from_existing(db, root, nonce, self.factories.clone()) .map_err(|_| CallError::StatePruned.to_string())?; let from = Address::default(); @@ -1477,10 +1481,21 @@ impl CallContract for Client { gas: U256::from(50_000_000), gas_price: U256::default(), value: U256::default(), - data: data, + data, }.fake_sign(from); - self.call(&transaction, Default::default(), &mut state, header) + let env_info = EnvInfo { + number: header.number(), + author: header.author().clone(), + timestamp: header.timestamp(), + difficulty: header.difficulty().clone(), + last_hashes: self.build_last_hashes(&parent.hash()), + gas_used: U256::default(), + gas_limit: U256::max_value(), + }; + let machine = self.engine.machine(); + + Self::do_virtual_call(&machine, &env_info, &mut state, &transaction, Default::default()) .map_err(|e| format!("{:?}", e)) .map(|executed| executed.output) } diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index b8f513d23d2..ebc0975580a 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -523,7 +523,10 @@ impl BlockInfo for TestBlockChainClient { impl CallContract for TestBlockChainClient { fn call_contract(&self, _id: BlockId, _address: Address, _data: Bytes) -> Result { Ok(vec![]) } - fn call_contract_at(&self, header: &Header, address: Address, data: Bytes) -> Result { Ok(vec![]) } + + fn call_contract_before(&self, _header: &Header, _address: Address, _data: Bytes) -> Result { + Ok(vec![]) + } } impl TransactionInfo for TestBlockChainClient { diff --git a/ethcore/src/engines/authority_round/mod.rs b/ethcore/src/engines/authority_round/mod.rs index 8e8f2c72199..d5da8c65135 100644 --- a/ethcore/src/engines/authority_round/mod.rs +++ b/ethcore/src/engines/authority_round/mod.rs @@ -1109,7 +1109,8 @@ 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(parent) { + if let Some(gas_limit) = self.gas_limit_override(header) { + trace!(target: "engine", "Setting gas limit to {} for block {}.", gas_limit, header.number()); header.set_gas_limit(gas_limit); } } @@ -1789,39 +1790,40 @@ impl Engine for AuthorityRound { finalized.into_iter().map(AncestryAction::MarkFinalized).collect() } - fn gas_limit_override(&self, parent: &Header) -> Option { - // TODO: Use target gas limit? Must exactly agree in all nodes! - let default_gas_limit = 10_000_000.into(); + fn gas_limit_override(&self, header: &Header) -> Option { let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) { Some(client) => client, None => { debug!(target: "engine", "Unable to prepare block: missing client ref."); - return Some(default_gas_limit); + return None; } }; let full_client = match client.as_full_client() { Some(full_client) => full_client, None => { debug!(target: "engine", "Failed to upgrade to BlockchainClient."); - return Some(default_gas_limit); + return None; } }; let address = match self.machine.tx_filter() { Some(tx_filter) => *tx_filter.contract_address(), None => { - debug!(target: "engine", "Not transaction filter configured. Not changing the block gas limit."); - return Some(default_gas_limit); + debug!(target: "engine", "No transaction filter configured. Not changing the block gas limit."); + return None; } }; - let (data, decoder) = transact_acl_gas_price::functions::limit_block_gas::call(); - match full_client.call_contract_at(parent, address, data).ok().and_then(|value| decoder.decode(&value).ok()) { - Some(true) => return Some(2_000_000.into()), - Some(false) => (), - None => debug!(target: "engine", "Failed to call limitBlockGas."), - }; - Some(default_gas_limit) + let (data, decoder) = transact_acl_gas_price::functions::block_gas_limit::call(); + let value = full_client.call_contract_before(header, address, data).map_err(|err| { + error!(target: "engine", "Failed to call blockGasLimit. Not changing the block gas limit. {:?}", err); + }).ok()?; + if value.is_empty() { + debug!(target: "engine", "blockGasLimit returned nothing. Not changing the block gas limit."); + None + } else { + decoder.decode(&value).ok() + } } } diff --git a/ethcore/src/engines/mod.rs b/ethcore/src/engines/mod.rs index ab977606fe9..cd729556175 100644 --- a/ethcore/src/engines/mod.rs +++ b/ethcore/src/engines/mod.rs @@ -460,7 +460,7 @@ pub trait Engine: Sync + Send { /// 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, parent: &Header) -> Option { + fn gas_limit_override(&self, _header: &Header) -> Option { None } } diff --git a/ethcore/src/verification/verification.rs b/ethcore/src/verification/verification.rs index 275264600a9..d191227f9ed 100644 --- a/ethcore/src/verification/verification.rs +++ b/ethcore/src/verification/verification.rs @@ -283,10 +283,13 @@ pub fn verify_header_params(header: &Header, engine: &EthEngine, is_full: bool, if header.gas_used() > header.gas_limit() { return Err(From::from(BlockError::TooMuchGasUsed(OutOfBounds { max: Some(*header.gas_limit()), min: None, found: *header.gas_used() }))); } - // TODO: This should be called with the _parent_ instead, and the value should be checked. - // Alternatively, only check in `verify_parent` below, and add another function to the engine that returns `true` if - // the engine uses a gas limit override. - if engine.gas_limit_override(header).is_none() { + if let Some(gas_limit) = engine.gas_limit_override(header) { + if *header.gas_limit() != gas_limit { + return Err(From::from(BlockError::InvalidGasLimit( + OutOfBounds { min: Some(gas_limit), max: Some(gas_limit), found: *header.gas_limit() } + ))); + } + } else { 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() }))); @@ -351,13 +354,7 @@ fn verify_parent(header: &Header, parent: &Header, engine: &EthEngine) -> Result return Err(BlockError::RidiculousNumber(OutOfBounds { min: Some(1), max: None, found: header.number() }).into()); } - if let Some(gas_limit) = engine.gas_limit_override(parent) { - if *header.gas_limit() != gas_limit { - return Err(From::from(BlockError::InvalidGasLimit( - OutOfBounds { min: Some(gas_limit), max: Some(gas_limit), found: *header.gas_limit() } - ))); - } - } else { + 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; From 07cbf846e637fbb54b640e05342cd7daac76d789 Mon Sep 17 00:00:00 2001 From: Andreas Fackler Date: Thu, 4 Jul 2019 14:03:01 +0200 Subject: [PATCH 3/4] Make the block gas limit contract a separate config option. --- ethcore/res/contracts/block_gas_limit.json | 16 +++++++++ ethcore/res/contracts/tx_acl_gas_price.json | 14 -------- ethcore/src/engines/authority_round/mod.rs | 15 +++----- ethcore/src/spec/spec.rs | 3 ++ json/src/spec/params.rs | 39 +++++++++++++++++++-- 5 files changed, 60 insertions(+), 27 deletions(-) create mode 100644 ethcore/res/contracts/block_gas_limit.json diff --git a/ethcore/res/contracts/block_gas_limit.json b/ethcore/res/contracts/block_gas_limit.json new file mode 100644 index 00000000000..bb2671b4571 --- /dev/null +++ b/ethcore/res/contracts/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/res/contracts/tx_acl_gas_price.json b/ethcore/res/contracts/tx_acl_gas_price.json index a8a2af22152..bc355dee7e6 100644 --- a/ethcore/res/contracts/tx_acl_gas_price.json +++ b/ethcore/res/contracts/tx_acl_gas_price.json @@ -41,20 +41,6 @@ "stateMutability": "view", "type": "function" }, - { - "constant": true, - "inputs": [], - "name": "blockGasLimit", - "outputs": [ - { - "name": "", - "type": "uint256" - } - ], - "payable": false, - "stateMutability": "view", - "type": "function" - }, { "constant": true, "inputs": [ diff --git a/ethcore/src/engines/authority_round/mod.rs b/ethcore/src/engines/authority_round/mod.rs index d5da8c65135..280eb6e473e 100644 --- a/ethcore/src/engines/authority_round/mod.rs +++ b/ethcore/src/engines/authority_round/mod.rs @@ -45,7 +45,6 @@ use itertools::{self, Itertools}; use rlp::{encode, Decodable, DecoderError, Encodable, RlpStream, Rlp}; use ethereum_types::{H256, H520, Address, U128, U256}; use parking_lot::{Mutex, RwLock}; -use tx_filter::transact_acl_gas_price; use types::BlockNumber; use types::ancestry_action::AncestryAction; use types::header::{Header, ExtendedHeader}; @@ -55,6 +54,8 @@ use unexpected::{Mismatch, OutOfBounds}; #[cfg(not(time_checked_add))] use time_utils::CheckedSystemTime; +use_contract!(block_gas_limit, "res/contracts/block_gas_limit.json"); + mod finality; mod randomness; pub(crate) mod util; @@ -1791,6 +1792,8 @@ impl Engine for AuthorityRound { } fn gas_limit_override(&self, header: &Header) -> Option { + let (_, &address) = self.machine.params().block_gas_limit_contract.range(..header.number()).last()?; + let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) { Some(client) => client, None => { @@ -1806,15 +1809,7 @@ impl Engine for AuthorityRound { } }; - let address = match self.machine.tx_filter() { - Some(tx_filter) => *tx_filter.contract_address(), - None => { - debug!(target: "engine", "No transaction filter configured. Not changing the block gas limit."); - return None; - } - }; - - let (data, decoder) = transact_acl_gas_price::functions::block_gas_limit::call(); + let (data, decoder) = block_gas_limit::functions::block_gas_limit::call(); let value = full_client.call_contract_before(header, address, data).map_err(|err| { error!(target: "engine", "Failed to call blockGasLimit. Not changing the block gas limit. {:?}", err); }).ok()?; diff --git a/ethcore/src/spec/spec.rs b/ethcore/src/spec/spec.rs index 4813fbc1040..ef18dcad76e 100644 --- a/ethcore/src/spec/spec.rs +++ b/ethcore/src/spec/spec.rs @@ -79,6 +79,8 @@ pub struct CommonParams { pub subprotocol_name: String, /// Minimum gas limit. pub min_gas_limit: U256, + /// The address of a contract that determines the block gas limit. + pub block_gas_limit_contract: BTreeMap, /// Fork block to check. pub fork_block: Option<(BlockNumber, H256)>, /// EIP150 transition block number. @@ -243,6 +245,7 @@ impl From for CommonParams { }, subprotocol_name: p.subprotocol_name.unwrap_or_else(|| "eth".to_owned()), min_gas_limit: p.min_gas_limit.into(), + block_gas_limit_contract: p.block_gas_limit_contract.map(|bglc| bglc.into()).unwrap_or_default(), fork_block: if let (Some(n), Some(h)) = (p.fork_block, p.fork_hash) { Some((n.into(), h.into())) } else { diff --git a/json/src/spec/params.rs b/json/src/spec/params.rs index 765384a6b91..5d381c5e602 100644 --- a/json/src/spec/params.rs +++ b/json/src/spec/params.rs @@ -16,6 +16,9 @@ //! Spec params deserialization. +use std::collections::BTreeMap; +use std::iter; +use ethereum_types::H160; use uint::{self, Uint}; use hash::{H256, Address}; use bytes::Bytes; @@ -31,6 +34,8 @@ pub struct Params { pub maximum_extra_data_size: Uint, /// Minimum gas limit. pub min_gas_limit: Uint, + /// The address of a contract that determines the block gas limit. + pub block_gas_limit_contract: Option, /// Network id. #[serde(rename = "networkID")] @@ -126,12 +131,36 @@ pub struct Params { pub kip6_transition: Option, } +/// A contract that determines block gas limits can be configured either as a single address, or as a map, assigning +/// to each starting block number the contract address that should be used from that block on. +#[derive(Debug, PartialEq, Deserialize)] +#[serde(deny_unknown_fields, untagged)] +pub enum BlockGasLimitContract { + /// A single address for a block gas limit contract. + Single(Address), + /// A map from block numbers to the contract addresses that should be used for the gas limit after that block. + Transitions(BTreeMap), +} + +impl From for BTreeMap { + fn from(contract: BlockGasLimitContract) -> Self { + match contract { + BlockGasLimitContract::Single(Address(addr)) => iter::once((0, addr)).collect(), + BlockGasLimitContract::Transitions(transitions) => { + transitions.into_iter().map(|(Uint(block), Address(addr))| (block.into(), addr)).collect() + } + } + } +} + #[cfg(test)] mod tests { + use std::str::FromStr; use serde_json; use uint::Uint; - use ethereum_types::U256; - use spec::params::Params; + use ethereum_types::{U256, H160}; + use hash::Address; + use spec::params::{BlockGasLimitContract, Params}; #[test] fn params_deserialization() { @@ -144,7 +173,11 @@ mod tests { "accountStartNonce": "0x01", "gasLimitBoundDivisor": "0x20", "maxCodeSize": "0x1000", - "wasmActivationTransition": "0x1010" + "wasmActivationTransition": "0x1010", + "blockGasLimitContract": { + "10": "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "20": "0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" + } }"#; let deserialized: Params = serde_json::from_str(s).unwrap(); From 1207d1e3bef46164d49b8be04271fc26e3befde8 Mon Sep 17 00:00:00 2001 From: Vadim Date: Mon, 15 Jul 2019 15:16:51 +0300 Subject: [PATCH 4/4] Add `info` level logging of block gas limit switching --- ethcore/src/engines/authority_round/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ethcore/src/engines/authority_round/mod.rs b/ethcore/src/engines/authority_round/mod.rs index 280eb6e473e..4803bac6e6e 100644 --- a/ethcore/src/engines/authority_round/mod.rs +++ b/ethcore/src/engines/authority_round/mod.rs @@ -1112,7 +1112,11 @@ impl Engine for AuthorityRound { 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); + } } } @@ -1792,7 +1796,7 @@ impl Engine for AuthorityRound { } fn gas_limit_override(&self, header: &Header) -> Option { - let (_, &address) = self.machine.params().block_gas_limit_contract.range(..header.number()).last()?; + let (_, &address) = self.machine.params().block_gas_limit_contract.range(..=header.number()).last()?; let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) { Some(client) => client,