From d7ea7678b288f9461949f1ac686653f649812829 Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Wed, 30 Jun 2021 11:42:21 -0400 Subject: [PATCH 01/38] Initial stab at EIP-2929 support --- gasometer/src/costs.rs | 65 ++++++++++---- gasometer/src/lib.rs | 171 ++++++++++++++++++++++++++---------- runtime/src/handler.rs | 7 ++ runtime/src/lib.rs | 57 ++++++++++++ src/executor/stack/mod.rs | 10 +++ src/executor/stack/state.rs | 10 +++ 6 files changed, 260 insertions(+), 60 deletions(-) diff --git a/gasometer/src/costs.rs b/gasometer/src/costs.rs index 674d6cc0b..cfe008634 100644 --- a/gasometer/src/costs.rs +++ b/gasometer/src/costs.rs @@ -113,11 +113,19 @@ pub fn verylowcopy_cost(len: U256) -> Result { Ok(gas.as_u64()) } -pub fn extcodecopy_cost(len: U256, config: &Config) -> Result { +pub fn extcodecopy_cost(len: U256, is_cold: bool, config: &Config) -> Result { let wordd = len / U256::from(32); let wordr = len % U256::from(32); + let gas_ext_code = match &config.eip_2929 { + None => config.gas_ext_code, + Some(eip_2929) => if is_cold { + eip_2929.cold_account_access_cost + } else { + eip_2929.warm_storage_read_cost + }, + }; - let gas = U256::from(config.gas_ext_code).checked_add( + let gas = U256::from(gas_ext_code).checked_add( U256::from(G_COPY).checked_mul( if wordr == U256::zero() { wordd @@ -169,37 +177,54 @@ pub fn sha3_cost(len: U256) -> Result { Ok(gas.as_u64()) } -pub fn sstore_cost(original: H256, current: H256, new: H256, gas: u64, config: &Config) -> Result { - if config.sstore_gas_metering { +pub fn sstore_cost(original: H256, current: H256, new: H256, gas: u64, is_cold: bool, config: &Config) -> Result { + // modify costs if EIP-2929 is enabled + let (gas_sload, gas_sstore_reset) = match &config.eip_2929 { + None => (config.gas_sload, config.gas_sstore_reset), + Some(eip_2929) => (eip_2929.warm_storage_read_cost, config.gas_sstore_reset - eip_2929.cold_sload_cost), + }; + let gas_cost = if config.sstore_gas_metering { if config.sstore_revert_under_stipend { if gas < config.call_stipend { return Err(ExitError::OutOfGas) } } - Ok(if new == current { - config.gas_sload + if new == current { + gas_sload } else { if original == current { if original == H256::zero() { config.gas_sstore_set } else { - config.gas_sstore_reset + gas_sstore_reset } } else { - config.gas_sload + gas_sload } - }) + } } else { - Ok(if current == H256::zero() && new != H256::zero() { + if current == H256::zero() && new != H256::zero() { config.gas_sstore_set } else { - config.gas_sstore_reset - }) - } + gas_sstore_reset + } + }; + Ok( + // In EIP-2929 we charge extra if the slot has not been used yet in this transaction + if is_cold { + config + .eip_2929 + .as_ref() + .map(|eip2929| gas_cost + eip2929.cold_sload_cost) + .unwrap_or(gas_cost) + } else { + gas_cost + } + ) } -pub fn suicide_cost(value: U256, target_exists: bool, config: &Config) -> u64 { +pub fn suicide_cost(value: U256, target_exists: bool, target_is_cold: bool, config: &Config) -> u64 { let eip161 = !config.empty_considered_exists; let should_charge_topup = if eip161 { value != U256::zero() && !target_exists @@ -213,7 +238,17 @@ pub fn suicide_cost(value: U256, target_exists: bool, config: &Config) -> u64 { 0 }; - config.gas_suicide + suicide_gas_topup + let gas_cost = config.gas_suicide + suicide_gas_topup; + // In EIP-2929 we charge extra if the recipient has not been used yet in this transaction + if target_is_cold { + config + .eip_2929 + .as_ref() + .map(|eip2929| gas_cost + eip2929.cold_account_access_cost) + .unwrap_or(gas_cost) + } else { + gas_cost + } } pub fn call_cost( diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 790852055..c0e09dc32 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -448,27 +448,42 @@ pub fn dynamic_opcode_cost( Opcode::SELFBALANCE if config.has_self_balance => GasCost::Low, Opcode::SELFBALANCE => GasCost::Invalid, - Opcode::EXTCODESIZE => GasCost::ExtCodeSize, - Opcode::BALANCE => GasCost::Balance, + Opcode::EXTCODESIZE => GasCost::ExtCodeSize { + is_cold: handler.is_cold(stack.peek(0)?.into(), None), + }, + Opcode::BALANCE => GasCost::Balance { + is_cold: handler.is_cold(stack.peek(0)?.into(), None), + }, Opcode::BLOCKHASH => GasCost::BlockHash, - Opcode::EXTCODEHASH if config.has_ext_code_hash => GasCost::ExtCodeHash, + Opcode::EXTCODEHASH if config.has_ext_code_hash => GasCost::ExtCodeHash { + is_cold: handler.is_cold(stack.peek(0)?.into(), None), + }, Opcode::EXTCODEHASH => GasCost::Invalid, - Opcode::CALLCODE => GasCost::CallCode { - value: U256::from_big_endian(&stack.peek(2)?[..]), - gas: U256::from_big_endian(&stack.peek(0)?[..]), - target_exists: handler.exists(stack.peek(1)?.into()), - }, - Opcode::STATICCALL => GasCost::StaticCall { - gas: U256::from_big_endian(&stack.peek(0)?[..]), - target_exists: handler.exists(stack.peek(1)?.into()), - }, + Opcode::CALLCODE => { + let target = stack.peek(1)?.into(); + GasCost::CallCode { + value: U256::from_big_endian(&stack.peek(2)?[..]), + gas: U256::from_big_endian(&stack.peek(0)?[..]), + target_exists: handler.exists(target), + target_is_cold: handler.is_cold(target, None), + } + } + Opcode::STATICCALL => { + let target = stack.peek(1)?.into(); + GasCost::StaticCall { + gas: U256::from_big_endian(&stack.peek(0)?[..]), + target_exists: handler.exists(target), + target_is_cold: handler.is_cold(target, None), + } + } Opcode::SHA3 => GasCost::Sha3 { len: U256::from_big_endian(&stack.peek(1)?[..]), }, Opcode::EXTCODECOPY => GasCost::ExtCodeCopy { len: U256::from_big_endian(&stack.peek(3)?[..]), + is_cold: handler.is_cold(stack.peek(0)?.into(), None), }, Opcode::CALLDATACOPY | Opcode::CODECOPY => GasCost::VeryLowCopy { len: U256::from_big_endian(&stack.peek(2)?[..]), @@ -476,12 +491,21 @@ pub fn dynamic_opcode_cost( Opcode::EXP => GasCost::Exp { power: U256::from_big_endian(&stack.peek(1)?[..]), }, - Opcode::SLOAD => GasCost::SLoad, + Opcode::SLOAD => { + let index = stack.peek(0)?; + GasCost::SLoad { + is_cold: handler.is_cold(address, Some(index)) + } + } - Opcode::DELEGATECALL if config.has_delegate_call => GasCost::DelegateCall { - gas: U256::from_big_endian(&stack.peek(0)?[..]), - target_exists: handler.exists(stack.peek(1)?.into()), - }, + Opcode::DELEGATECALL if config.has_delegate_call => { + let target = stack.peek(1)?.into(); + GasCost::DelegateCall { + gas: U256::from_big_endian(&stack.peek(0)?[..]), + target_exists: handler.exists(target), + target_is_cold: handler.is_cold(target, None), + } + } Opcode::DELEGATECALL => GasCost::Invalid, Opcode::RETURNDATASIZE if config.has_return_data => GasCost::Base, @@ -493,11 +517,13 @@ pub fn dynamic_opcode_cost( Opcode::SSTORE if !is_static => { let index = stack.peek(0)?; let value = stack.peek(1)?; + let is_cold = handler.is_cold(address, Some(index)); GasCost::SStore { original: handler.original_storage(address, index), current: handler.storage(address, index), new: value, + is_cold, } }, Opcode::LOG0 if !is_static => GasCost::Log { @@ -524,19 +550,26 @@ pub fn dynamic_opcode_cost( Opcode::CREATE2 if !is_static && config.has_create2 => GasCost::Create2 { len: U256::from_big_endian(&stack.peek(2)?[..]), }, - Opcode::SUICIDE if !is_static => GasCost::Suicide { - value: handler.balance(address), - target_exists: handler.exists(stack.peek(0)?.into()), - already_removed: handler.deleted(address), - }, + Opcode::SUICIDE if !is_static => { + let target = stack.peek(0)?.into(); + GasCost::Suicide { + value: handler.balance(address), + target_exists: handler.exists(target), + target_is_cold: handler.is_cold(target, None), + already_removed: handler.deleted(address), + } + } Opcode::CALL if !is_static || - (is_static && U256::from_big_endian(&stack.peek(2)?[..]) == U256::zero()) => + (is_static && U256::from_big_endian(&stack.peek(2)?[..]) == U256::zero()) => { + let target = stack.peek(1)?.into(); GasCost::Call { value: U256::from_big_endian(&stack.peek(2)?[..]), gas: U256::from_big_endian(&stack.peek(0)?[..]), - target_exists: handler.exists(stack.peek(1)?.into()), - }, + target_exists: handler.exists(target), + target_is_cold: handler.is_cold(target, None), + } + } _ => GasCost::Invalid, }; @@ -665,20 +698,28 @@ impl<'config> Inner<'config> { costs::call_cost(U256::zero(), false, false, !target_exists, self.config), GasCost::StaticCall { target_exists, .. } => costs::call_cost(U256::zero(), false, true, !target_exists, self.config), - GasCost::Suicide { value, target_exists, .. } => - costs::suicide_cost(value, target_exists, self.config), + GasCost::Suicide { value, target_exists, target_is_cold, .. } => + costs::suicide_cost(value, target_exists, target_is_cold, self.config), GasCost::SStore { .. } if self.config.estimate => self.config.gas_sstore_set, - GasCost::SStore { original, current, new } => - costs::sstore_cost(original, current, new, gas, self.config)?, + GasCost::SStore { original, current, new, is_cold } => + costs::sstore_cost(original, current, new, gas, is_cold, self.config)?, GasCost::Sha3 { len } => costs::sha3_cost(len)?, GasCost::Log { n, len } => costs::log_cost(n, len)?, - GasCost::ExtCodeCopy { len } => costs::extcodecopy_cost(len, self.config)?, + GasCost::ExtCodeCopy { len, is_cold } => + costs::extcodecopy_cost(len, is_cold, self.config)?, GasCost::VeryLowCopy { len } => costs::verylowcopy_cost(len)?, GasCost::Exp { power } => costs::exp_cost(power, self.config)?, GasCost::Create => consts::G_CREATE, GasCost::Create2 { len } => costs::create2_cost(len)?, - GasCost::SLoad => self.config.gas_sload, + GasCost::SLoad { is_cold } => match &self.config.eip_2929 { + None => self.config.gas_sload, + Some(eip_2929) => if is_cold { + eip_2929.cold_sload_cost + } else { + eip_2929.warm_storage_read_cost + }, + }, GasCost::Zero => consts::G_ZERO, GasCost::Base => consts::G_BASE, @@ -686,13 +727,27 @@ impl<'config> Inner<'config> { GasCost::Low => consts::G_LOW, GasCost::Invalid => return Err(ExitError::OutOfGas), - GasCost::ExtCodeSize => self.config.gas_ext_code, - GasCost::Balance => self.config.gas_balance, + GasCost::ExtCodeSize { is_cold } => + self.account_access_cost(is_cold, self.config.gas_ext_code), + GasCost::Balance { is_cold } => + self.account_access_cost(is_cold, self.config.gas_balance), GasCost::BlockHash => consts::G_BLOCKHASH, - GasCost::ExtCodeHash => self.config.gas_ext_code_hash, + GasCost::ExtCodeHash { is_cold } => + self.account_access_cost(is_cold, self.config.gas_ext_code_hash), }) } + fn account_access_cost(&self, is_cold: bool, default_value: u64) -> u64 { + match &self.config.eip_2929 { + None => default_value, + Some(eip_2929) => if is_cold { + eip_2929.cold_account_access_cost + } else { + eip_2929.warm_storage_read_cost + }, + } + } + fn gas_refund( &self, cost: GasCost @@ -700,7 +755,7 @@ impl<'config> Inner<'config> { match cost { _ if self.config.estimate => 0, - GasCost::SStore { original, current, new } => + GasCost::SStore { original, current, new, .. } => costs::sstore_refund(original, current, new, self.config), GasCost::Suicide { already_removed, .. } => costs::suicide_refund(already_removed), @@ -724,13 +779,22 @@ pub enum GasCost { Invalid, /// Gas cost for `EXTCODESIZE`. - ExtCodeSize, + ExtCodeSize { + /// True if address has not been previously accessed in this transaction + is_cold: bool, + }, /// Gas cost for `BALANCE`. - Balance, + Balance { + /// True if address has not been previously accessed in this transaction + is_cold: bool, + }, /// Gas cost for `BLOCKHASH`. BlockHash, /// Gas cost for `EXTBLOCKHASH`. - ExtCodeHash, + ExtCodeHash { + /// True if address has not been previously accessed in this transaction + is_cold: bool, + }, /// Gas cost for `CALL`. Call { @@ -739,7 +803,9 @@ pub enum GasCost { /// Call gas. gas: U256, /// Whether the target exists. - target_exists: bool + target_exists: bool, + /// True if target has not been previously accessed in this transaction + target_is_cold: bool, }, /// Gas cost for `CALLCODE. CallCode { @@ -748,21 +814,27 @@ pub enum GasCost { /// Call gas. gas: U256, /// Whether the target exists. - target_exists: bool + target_exists: bool, + /// True if target has not been previously accessed in this transaction + target_is_cold: bool, }, /// Gas cost for `DELEGATECALL`. DelegateCall { /// Call gas. gas: U256, /// Whether the target exists. - target_exists: bool + target_exists: bool, + /// True if target has not been previously accessed in this transaction + target_is_cold: bool, }, /// Gas cost for `STATICCALL`. StaticCall { /// Call gas. gas: U256, /// Whether the target exists. - target_exists: bool + target_exists: bool, + /// True if target has not been previously accessed in this transaction + target_is_cold: bool, }, /// Gas cost for `SUICIDE`. Suicide { @@ -770,6 +842,8 @@ pub enum GasCost { value: U256, /// Whether the target exists. target_exists: bool, + /// True if target has not been previously accessed in this transaction + target_is_cold: bool, /// Whether the target has already been removed. already_removed: bool }, @@ -780,7 +854,9 @@ pub enum GasCost { /// Current value. current: H256, /// New value. - new: H256 + new: H256, + /// True if slot has not been previously accessed in this transaction + is_cold: bool, }, /// Gas cost for `SHA3`. Sha3 { @@ -797,7 +873,9 @@ pub enum GasCost { /// Gas cost for `EXTCODECOPY`. ExtCodeCopy { /// Length. - len: U256 + len: U256, + /// True if address has not been previously accessed in this transaction + is_cold: bool, }, /// Gas cost for some copy opcodes that is documented as `VERYLOW`. VeryLowCopy { @@ -817,7 +895,10 @@ pub enum GasCost { len: U256 }, /// Gas cost for `SLOAD`. - SLoad, + SLoad { + /// True if slot has not been previously accessed in this transaction + is_cold: bool, + }, } /// Memory cost. diff --git a/runtime/src/handler.rs b/runtime/src/handler.rs index 0177daa92..4616ed22c 100644 --- a/runtime/src/handler.rs +++ b/runtime/src/handler.rs @@ -63,6 +63,13 @@ pub trait Handler { fn exists(&self, address: H160) -> bool; /// Check whether an address has already been deleted. fn deleted(&self, address: H160) -> bool; + /// Checks if the address or (address, index) pair has been previously accessed + /// (or set in `accessed_addresses` / `accessed_storage_keys` via an access list + /// transaction). + /// References: + /// * https://eips.ethereum.org/EIPS/eip-2929 + /// * https://eips.ethereum.org/EIPS/eip-2930 + fn is_cold(&self, address: H160, index: Option) -> bool; /// Set storage value of address at index. fn set_storage(&mut self, address: H160, index: H256, value: H256) -> Result<(), ExitError>; diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 09e355f29..637bc9858 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -161,6 +161,14 @@ impl<'config> Runtime<'config> { } } +/// Parameters specific to EIP-2929 (see https://eips.ethereum.org/EIPS/eip-2929) +#[derive(Clone, Debug)] +pub struct EIP2929Config { + pub cold_sload_cost: u64, + pub cold_account_access_cost: u64, + pub warm_storage_read_cost: u64, +} + /// Runtime configuration. #[derive(Clone, Debug)] pub struct Config { @@ -236,6 +244,7 @@ pub struct Config { pub has_ext_code_hash: bool, /// Whether the gasometer is running in estimate mode. pub estimate: bool, + pub eip_2929: Option, } impl Config { @@ -277,6 +286,7 @@ impl Config { has_self_balance: false, has_ext_code_hash: false, estimate: false, + eip_2929: None, } } @@ -318,6 +328,53 @@ impl Config { has_self_balance: true, has_ext_code_hash: true, estimate: false, + eip_2929: None, + } + } + + /// Berlin hard fork configuration. + pub const fn berlin() -> Config { + Config { + gas_ext_code: 700, + gas_ext_code_hash: 700, + gas_balance: 700, + gas_sload: 800, + gas_sstore_set: 20000, + gas_sstore_reset: 5000, + refund_sstore_clears: 15000, + gas_suicide: 5000, + gas_suicide_new_account: 25000, + gas_call: 700, + gas_expbyte: 50, + gas_transaction_create: 53000, + gas_transaction_call: 21000, + gas_transaction_zero_data: 4, + gas_transaction_non_zero_data: 16, + sstore_gas_metering: true, + sstore_revert_under_stipend: true, + err_on_call_with_more_gas: false, + empty_considered_exists: false, + create_increase_nonce: true, + call_l64_after_gas: true, + stack_limit: 1024, + memory_limit: usize::max_value(), + call_stack_limit: 1024, + create_contract_limit: Some(0x6000), + call_stipend: 2300, + has_delegate_call: true, + has_create2: true, + has_revert: true, + has_return_data: true, + has_bitwise_shifting: true, + has_chain_id: true, + has_self_balance: true, + has_ext_code_hash: true, + estimate: false, + eip_2929: Some(EIP2929Config { + cold_sload_cost: 2100, + cold_account_access_cost: 2600, + warm_storage_read_cost: 100, + }), } } } diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 522ad80b9..5474f391f 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -658,6 +658,16 @@ impl<'config, S: StackState<'config>> Handler for StackExecutor<'config, S> { } } + fn is_cold(&self, address: H160, maybe_index: Option) -> bool { + // TODO: how to ensure the correct initialization? + // From spec: "`accessed_addresses` is initialized to include: `tx.sender`, `tx.to` and + // the set of all precompiles." + match maybe_index { + None => self.state.is_known(address), + Some(index) => self.state.is_storage_known(address, index), + } + } + fn gas_left(&self) -> U256 { U256::from(self.state.metadata().gasometer.gas()) } diff --git a/src/executor/stack/state.rs b/src/executor/stack/state.rs index dabb1d440..469e1d87d 100644 --- a/src/executor/stack/state.rs +++ b/src/executor/stack/state.rs @@ -354,6 +354,8 @@ pub trait StackState<'config>: Backend { fn is_empty(&self, address: H160) -> bool; fn deleted(&self, address: H160) -> bool; + fn is_known(&self, address: H160) -> bool; + fn is_storage_known(&self, address: H160, key: H256) -> bool; fn inc_nonce(&mut self, address: H160); fn set_storage(&mut self, address: H160, key: H256, value: H256); @@ -447,6 +449,14 @@ impl<'backend, 'config, B: Backend> StackState<'config> for MemoryStackState<'ba self.substate.deleted(address) } + fn is_known(&self, address: H160) -> bool { + self.substate.known_account(address).is_some() + } + + fn is_storage_known(&self, address: H160, key: H256) -> bool { + self.substate.known_storage(address, key).is_some() + } + fn inc_nonce(&mut self, address: H160) { self.substate.inc_nonce(address, self.backend); } From 8c24c13802c2064c77c2148cf2e209663bd75d0e Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Wed, 30 Jun 2021 15:14:37 -0400 Subject: [PATCH 02/38] Revert "Initial stab at EIP-2929 support" This reverts commit d7ea7678b288f9461949f1ac686653f649812829. --- gasometer/src/costs.rs | 65 ++++---------- gasometer/src/lib.rs | 171 ++++++++++-------------------------- runtime/src/handler.rs | 7 -- runtime/src/lib.rs | 57 ------------ src/executor/stack/mod.rs | 10 --- src/executor/stack/state.rs | 10 --- 6 files changed, 60 insertions(+), 260 deletions(-) diff --git a/gasometer/src/costs.rs b/gasometer/src/costs.rs index cfe008634..674d6cc0b 100644 --- a/gasometer/src/costs.rs +++ b/gasometer/src/costs.rs @@ -113,19 +113,11 @@ pub fn verylowcopy_cost(len: U256) -> Result { Ok(gas.as_u64()) } -pub fn extcodecopy_cost(len: U256, is_cold: bool, config: &Config) -> Result { +pub fn extcodecopy_cost(len: U256, config: &Config) -> Result { let wordd = len / U256::from(32); let wordr = len % U256::from(32); - let gas_ext_code = match &config.eip_2929 { - None => config.gas_ext_code, - Some(eip_2929) => if is_cold { - eip_2929.cold_account_access_cost - } else { - eip_2929.warm_storage_read_cost - }, - }; - let gas = U256::from(gas_ext_code).checked_add( + let gas = U256::from(config.gas_ext_code).checked_add( U256::from(G_COPY).checked_mul( if wordr == U256::zero() { wordd @@ -177,54 +169,37 @@ pub fn sha3_cost(len: U256) -> Result { Ok(gas.as_u64()) } -pub fn sstore_cost(original: H256, current: H256, new: H256, gas: u64, is_cold: bool, config: &Config) -> Result { - // modify costs if EIP-2929 is enabled - let (gas_sload, gas_sstore_reset) = match &config.eip_2929 { - None => (config.gas_sload, config.gas_sstore_reset), - Some(eip_2929) => (eip_2929.warm_storage_read_cost, config.gas_sstore_reset - eip_2929.cold_sload_cost), - }; - let gas_cost = if config.sstore_gas_metering { +pub fn sstore_cost(original: H256, current: H256, new: H256, gas: u64, config: &Config) -> Result { + if config.sstore_gas_metering { if config.sstore_revert_under_stipend { if gas < config.call_stipend { return Err(ExitError::OutOfGas) } } - if new == current { - gas_sload + Ok(if new == current { + config.gas_sload } else { if original == current { if original == H256::zero() { config.gas_sstore_set } else { - gas_sstore_reset + config.gas_sstore_reset } } else { - gas_sload + config.gas_sload } - } + }) } else { - if current == H256::zero() && new != H256::zero() { + Ok(if current == H256::zero() && new != H256::zero() { config.gas_sstore_set } else { - gas_sstore_reset - } - }; - Ok( - // In EIP-2929 we charge extra if the slot has not been used yet in this transaction - if is_cold { - config - .eip_2929 - .as_ref() - .map(|eip2929| gas_cost + eip2929.cold_sload_cost) - .unwrap_or(gas_cost) - } else { - gas_cost - } - ) + config.gas_sstore_reset + }) + } } -pub fn suicide_cost(value: U256, target_exists: bool, target_is_cold: bool, config: &Config) -> u64 { +pub fn suicide_cost(value: U256, target_exists: bool, config: &Config) -> u64 { let eip161 = !config.empty_considered_exists; let should_charge_topup = if eip161 { value != U256::zero() && !target_exists @@ -238,17 +213,7 @@ pub fn suicide_cost(value: U256, target_exists: bool, target_is_cold: bool, conf 0 }; - let gas_cost = config.gas_suicide + suicide_gas_topup; - // In EIP-2929 we charge extra if the recipient has not been used yet in this transaction - if target_is_cold { - config - .eip_2929 - .as_ref() - .map(|eip2929| gas_cost + eip2929.cold_account_access_cost) - .unwrap_or(gas_cost) - } else { - gas_cost - } + config.gas_suicide + suicide_gas_topup } pub fn call_cost( diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index c0e09dc32..790852055 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -448,42 +448,27 @@ pub fn dynamic_opcode_cost( Opcode::SELFBALANCE if config.has_self_balance => GasCost::Low, Opcode::SELFBALANCE => GasCost::Invalid, - Opcode::EXTCODESIZE => GasCost::ExtCodeSize { - is_cold: handler.is_cold(stack.peek(0)?.into(), None), - }, - Opcode::BALANCE => GasCost::Balance { - is_cold: handler.is_cold(stack.peek(0)?.into(), None), - }, + Opcode::EXTCODESIZE => GasCost::ExtCodeSize, + Opcode::BALANCE => GasCost::Balance, Opcode::BLOCKHASH => GasCost::BlockHash, - Opcode::EXTCODEHASH if config.has_ext_code_hash => GasCost::ExtCodeHash { - is_cold: handler.is_cold(stack.peek(0)?.into(), None), - }, + Opcode::EXTCODEHASH if config.has_ext_code_hash => GasCost::ExtCodeHash, Opcode::EXTCODEHASH => GasCost::Invalid, - Opcode::CALLCODE => { - let target = stack.peek(1)?.into(); - GasCost::CallCode { - value: U256::from_big_endian(&stack.peek(2)?[..]), - gas: U256::from_big_endian(&stack.peek(0)?[..]), - target_exists: handler.exists(target), - target_is_cold: handler.is_cold(target, None), - } - } - Opcode::STATICCALL => { - let target = stack.peek(1)?.into(); - GasCost::StaticCall { - gas: U256::from_big_endian(&stack.peek(0)?[..]), - target_exists: handler.exists(target), - target_is_cold: handler.is_cold(target, None), - } - } + Opcode::CALLCODE => GasCost::CallCode { + value: U256::from_big_endian(&stack.peek(2)?[..]), + gas: U256::from_big_endian(&stack.peek(0)?[..]), + target_exists: handler.exists(stack.peek(1)?.into()), + }, + Opcode::STATICCALL => GasCost::StaticCall { + gas: U256::from_big_endian(&stack.peek(0)?[..]), + target_exists: handler.exists(stack.peek(1)?.into()), + }, Opcode::SHA3 => GasCost::Sha3 { len: U256::from_big_endian(&stack.peek(1)?[..]), }, Opcode::EXTCODECOPY => GasCost::ExtCodeCopy { len: U256::from_big_endian(&stack.peek(3)?[..]), - is_cold: handler.is_cold(stack.peek(0)?.into(), None), }, Opcode::CALLDATACOPY | Opcode::CODECOPY => GasCost::VeryLowCopy { len: U256::from_big_endian(&stack.peek(2)?[..]), @@ -491,21 +476,12 @@ pub fn dynamic_opcode_cost( Opcode::EXP => GasCost::Exp { power: U256::from_big_endian(&stack.peek(1)?[..]), }, - Opcode::SLOAD => { - let index = stack.peek(0)?; - GasCost::SLoad { - is_cold: handler.is_cold(address, Some(index)) - } - } + Opcode::SLOAD => GasCost::SLoad, - Opcode::DELEGATECALL if config.has_delegate_call => { - let target = stack.peek(1)?.into(); - GasCost::DelegateCall { - gas: U256::from_big_endian(&stack.peek(0)?[..]), - target_exists: handler.exists(target), - target_is_cold: handler.is_cold(target, None), - } - } + Opcode::DELEGATECALL if config.has_delegate_call => GasCost::DelegateCall { + gas: U256::from_big_endian(&stack.peek(0)?[..]), + target_exists: handler.exists(stack.peek(1)?.into()), + }, Opcode::DELEGATECALL => GasCost::Invalid, Opcode::RETURNDATASIZE if config.has_return_data => GasCost::Base, @@ -517,13 +493,11 @@ pub fn dynamic_opcode_cost( Opcode::SSTORE if !is_static => { let index = stack.peek(0)?; let value = stack.peek(1)?; - let is_cold = handler.is_cold(address, Some(index)); GasCost::SStore { original: handler.original_storage(address, index), current: handler.storage(address, index), new: value, - is_cold, } }, Opcode::LOG0 if !is_static => GasCost::Log { @@ -550,26 +524,19 @@ pub fn dynamic_opcode_cost( Opcode::CREATE2 if !is_static && config.has_create2 => GasCost::Create2 { len: U256::from_big_endian(&stack.peek(2)?[..]), }, - Opcode::SUICIDE if !is_static => { - let target = stack.peek(0)?.into(); - GasCost::Suicide { - value: handler.balance(address), - target_exists: handler.exists(target), - target_is_cold: handler.is_cold(target, None), - already_removed: handler.deleted(address), - } - } + Opcode::SUICIDE if !is_static => GasCost::Suicide { + value: handler.balance(address), + target_exists: handler.exists(stack.peek(0)?.into()), + already_removed: handler.deleted(address), + }, Opcode::CALL if !is_static || - (is_static && U256::from_big_endian(&stack.peek(2)?[..]) == U256::zero()) => { - let target = stack.peek(1)?.into(); + (is_static && U256::from_big_endian(&stack.peek(2)?[..]) == U256::zero()) => GasCost::Call { value: U256::from_big_endian(&stack.peek(2)?[..]), gas: U256::from_big_endian(&stack.peek(0)?[..]), - target_exists: handler.exists(target), - target_is_cold: handler.is_cold(target, None), - } - } + target_exists: handler.exists(stack.peek(1)?.into()), + }, _ => GasCost::Invalid, }; @@ -698,28 +665,20 @@ impl<'config> Inner<'config> { costs::call_cost(U256::zero(), false, false, !target_exists, self.config), GasCost::StaticCall { target_exists, .. } => costs::call_cost(U256::zero(), false, true, !target_exists, self.config), - GasCost::Suicide { value, target_exists, target_is_cold, .. } => - costs::suicide_cost(value, target_exists, target_is_cold, self.config), + GasCost::Suicide { value, target_exists, .. } => + costs::suicide_cost(value, target_exists, self.config), GasCost::SStore { .. } if self.config.estimate => self.config.gas_sstore_set, - GasCost::SStore { original, current, new, is_cold } => - costs::sstore_cost(original, current, new, gas, is_cold, self.config)?, + GasCost::SStore { original, current, new } => + costs::sstore_cost(original, current, new, gas, self.config)?, GasCost::Sha3 { len } => costs::sha3_cost(len)?, GasCost::Log { n, len } => costs::log_cost(n, len)?, - GasCost::ExtCodeCopy { len, is_cold } => - costs::extcodecopy_cost(len, is_cold, self.config)?, + GasCost::ExtCodeCopy { len } => costs::extcodecopy_cost(len, self.config)?, GasCost::VeryLowCopy { len } => costs::verylowcopy_cost(len)?, GasCost::Exp { power } => costs::exp_cost(power, self.config)?, GasCost::Create => consts::G_CREATE, GasCost::Create2 { len } => costs::create2_cost(len)?, - GasCost::SLoad { is_cold } => match &self.config.eip_2929 { - None => self.config.gas_sload, - Some(eip_2929) => if is_cold { - eip_2929.cold_sload_cost - } else { - eip_2929.warm_storage_read_cost - }, - }, + GasCost::SLoad => self.config.gas_sload, GasCost::Zero => consts::G_ZERO, GasCost::Base => consts::G_BASE, @@ -727,27 +686,13 @@ impl<'config> Inner<'config> { GasCost::Low => consts::G_LOW, GasCost::Invalid => return Err(ExitError::OutOfGas), - GasCost::ExtCodeSize { is_cold } => - self.account_access_cost(is_cold, self.config.gas_ext_code), - GasCost::Balance { is_cold } => - self.account_access_cost(is_cold, self.config.gas_balance), + GasCost::ExtCodeSize => self.config.gas_ext_code, + GasCost::Balance => self.config.gas_balance, GasCost::BlockHash => consts::G_BLOCKHASH, - GasCost::ExtCodeHash { is_cold } => - self.account_access_cost(is_cold, self.config.gas_ext_code_hash), + GasCost::ExtCodeHash => self.config.gas_ext_code_hash, }) } - fn account_access_cost(&self, is_cold: bool, default_value: u64) -> u64 { - match &self.config.eip_2929 { - None => default_value, - Some(eip_2929) => if is_cold { - eip_2929.cold_account_access_cost - } else { - eip_2929.warm_storage_read_cost - }, - } - } - fn gas_refund( &self, cost: GasCost @@ -755,7 +700,7 @@ impl<'config> Inner<'config> { match cost { _ if self.config.estimate => 0, - GasCost::SStore { original, current, new, .. } => + GasCost::SStore { original, current, new } => costs::sstore_refund(original, current, new, self.config), GasCost::Suicide { already_removed, .. } => costs::suicide_refund(already_removed), @@ -779,22 +724,13 @@ pub enum GasCost { Invalid, /// Gas cost for `EXTCODESIZE`. - ExtCodeSize { - /// True if address has not been previously accessed in this transaction - is_cold: bool, - }, + ExtCodeSize, /// Gas cost for `BALANCE`. - Balance { - /// True if address has not been previously accessed in this transaction - is_cold: bool, - }, + Balance, /// Gas cost for `BLOCKHASH`. BlockHash, /// Gas cost for `EXTBLOCKHASH`. - ExtCodeHash { - /// True if address has not been previously accessed in this transaction - is_cold: bool, - }, + ExtCodeHash, /// Gas cost for `CALL`. Call { @@ -803,9 +739,7 @@ pub enum GasCost { /// Call gas. gas: U256, /// Whether the target exists. - target_exists: bool, - /// True if target has not been previously accessed in this transaction - target_is_cold: bool, + target_exists: bool }, /// Gas cost for `CALLCODE. CallCode { @@ -814,27 +748,21 @@ pub enum GasCost { /// Call gas. gas: U256, /// Whether the target exists. - target_exists: bool, - /// True if target has not been previously accessed in this transaction - target_is_cold: bool, + target_exists: bool }, /// Gas cost for `DELEGATECALL`. DelegateCall { /// Call gas. gas: U256, /// Whether the target exists. - target_exists: bool, - /// True if target has not been previously accessed in this transaction - target_is_cold: bool, + target_exists: bool }, /// Gas cost for `STATICCALL`. StaticCall { /// Call gas. gas: U256, /// Whether the target exists. - target_exists: bool, - /// True if target has not been previously accessed in this transaction - target_is_cold: bool, + target_exists: bool }, /// Gas cost for `SUICIDE`. Suicide { @@ -842,8 +770,6 @@ pub enum GasCost { value: U256, /// Whether the target exists. target_exists: bool, - /// True if target has not been previously accessed in this transaction - target_is_cold: bool, /// Whether the target has already been removed. already_removed: bool }, @@ -854,9 +780,7 @@ pub enum GasCost { /// Current value. current: H256, /// New value. - new: H256, - /// True if slot has not been previously accessed in this transaction - is_cold: bool, + new: H256 }, /// Gas cost for `SHA3`. Sha3 { @@ -873,9 +797,7 @@ pub enum GasCost { /// Gas cost for `EXTCODECOPY`. ExtCodeCopy { /// Length. - len: U256, - /// True if address has not been previously accessed in this transaction - is_cold: bool, + len: U256 }, /// Gas cost for some copy opcodes that is documented as `VERYLOW`. VeryLowCopy { @@ -895,10 +817,7 @@ pub enum GasCost { len: U256 }, /// Gas cost for `SLOAD`. - SLoad { - /// True if slot has not been previously accessed in this transaction - is_cold: bool, - }, + SLoad, } /// Memory cost. diff --git a/runtime/src/handler.rs b/runtime/src/handler.rs index 4616ed22c..0177daa92 100644 --- a/runtime/src/handler.rs +++ b/runtime/src/handler.rs @@ -63,13 +63,6 @@ pub trait Handler { fn exists(&self, address: H160) -> bool; /// Check whether an address has already been deleted. fn deleted(&self, address: H160) -> bool; - /// Checks if the address or (address, index) pair has been previously accessed - /// (or set in `accessed_addresses` / `accessed_storage_keys` via an access list - /// transaction). - /// References: - /// * https://eips.ethereum.org/EIPS/eip-2929 - /// * https://eips.ethereum.org/EIPS/eip-2930 - fn is_cold(&self, address: H160, index: Option) -> bool; /// Set storage value of address at index. fn set_storage(&mut self, address: H160, index: H256, value: H256) -> Result<(), ExitError>; diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 637bc9858..09e355f29 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -161,14 +161,6 @@ impl<'config> Runtime<'config> { } } -/// Parameters specific to EIP-2929 (see https://eips.ethereum.org/EIPS/eip-2929) -#[derive(Clone, Debug)] -pub struct EIP2929Config { - pub cold_sload_cost: u64, - pub cold_account_access_cost: u64, - pub warm_storage_read_cost: u64, -} - /// Runtime configuration. #[derive(Clone, Debug)] pub struct Config { @@ -244,7 +236,6 @@ pub struct Config { pub has_ext_code_hash: bool, /// Whether the gasometer is running in estimate mode. pub estimate: bool, - pub eip_2929: Option, } impl Config { @@ -286,7 +277,6 @@ impl Config { has_self_balance: false, has_ext_code_hash: false, estimate: false, - eip_2929: None, } } @@ -328,53 +318,6 @@ impl Config { has_self_balance: true, has_ext_code_hash: true, estimate: false, - eip_2929: None, - } - } - - /// Berlin hard fork configuration. - pub const fn berlin() -> Config { - Config { - gas_ext_code: 700, - gas_ext_code_hash: 700, - gas_balance: 700, - gas_sload: 800, - gas_sstore_set: 20000, - gas_sstore_reset: 5000, - refund_sstore_clears: 15000, - gas_suicide: 5000, - gas_suicide_new_account: 25000, - gas_call: 700, - gas_expbyte: 50, - gas_transaction_create: 53000, - gas_transaction_call: 21000, - gas_transaction_zero_data: 4, - gas_transaction_non_zero_data: 16, - sstore_gas_metering: true, - sstore_revert_under_stipend: true, - err_on_call_with_more_gas: false, - empty_considered_exists: false, - create_increase_nonce: true, - call_l64_after_gas: true, - stack_limit: 1024, - memory_limit: usize::max_value(), - call_stack_limit: 1024, - create_contract_limit: Some(0x6000), - call_stipend: 2300, - has_delegate_call: true, - has_create2: true, - has_revert: true, - has_return_data: true, - has_bitwise_shifting: true, - has_chain_id: true, - has_self_balance: true, - has_ext_code_hash: true, - estimate: false, - eip_2929: Some(EIP2929Config { - cold_sload_cost: 2100, - cold_account_access_cost: 2600, - warm_storage_read_cost: 100, - }), } } } diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 5474f391f..522ad80b9 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -658,16 +658,6 @@ impl<'config, S: StackState<'config>> Handler for StackExecutor<'config, S> { } } - fn is_cold(&self, address: H160, maybe_index: Option) -> bool { - // TODO: how to ensure the correct initialization? - // From spec: "`accessed_addresses` is initialized to include: `tx.sender`, `tx.to` and - // the set of all precompiles." - match maybe_index { - None => self.state.is_known(address), - Some(index) => self.state.is_storage_known(address, index), - } - } - fn gas_left(&self) -> U256 { U256::from(self.state.metadata().gasometer.gas()) } diff --git a/src/executor/stack/state.rs b/src/executor/stack/state.rs index 469e1d87d..dabb1d440 100644 --- a/src/executor/stack/state.rs +++ b/src/executor/stack/state.rs @@ -354,8 +354,6 @@ pub trait StackState<'config>: Backend { fn is_empty(&self, address: H160) -> bool; fn deleted(&self, address: H160) -> bool; - fn is_known(&self, address: H160) -> bool; - fn is_storage_known(&self, address: H160, key: H256) -> bool; fn inc_nonce(&mut self, address: H160); fn set_storage(&mut self, address: H160, key: H256, value: H256); @@ -449,14 +447,6 @@ impl<'backend, 'config, B: Backend> StackState<'config> for MemoryStackState<'ba self.substate.deleted(address) } - fn is_known(&self, address: H160) -> bool { - self.substate.known_account(address).is_some() - } - - fn is_storage_known(&self, address: H160, key: H256) -> bool { - self.substate.known_storage(address, key).is_some() - } - fn inc_nonce(&mut self, address: H160) { self.substate.inc_nonce(address, self.backend); } From b471064011810787960df22194888d5927c6060d Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Wed, 30 Jun 2021 16:38:19 -0400 Subject: [PATCH 03/38] Another try at EIP-2929 (I think this one is better) --- gasometer/src/costs.rs | 97 +++++++++++++++--- gasometer/src/lib.rs | 203 ++++++++++++++++++++++++++++---------- runtime/src/lib.rs | 57 +++++++++++ src/executor/stack/mod.rs | 28 +++++- 4 files changed, 316 insertions(+), 69 deletions(-) diff --git a/gasometer/src/costs.rs b/gasometer/src/costs.rs index 674d6cc0b..09a25cc4f 100644 --- a/gasometer/src/costs.rs +++ b/gasometer/src/costs.rs @@ -113,11 +113,19 @@ pub fn verylowcopy_cost(len: U256) -> Result { Ok(gas.as_u64()) } -pub fn extcodecopy_cost(len: U256, config: &Config) -> Result { +pub fn extcodecopy_cost(len: U256, is_cold: bool, config: &Config) -> Result { let wordd = len / U256::from(32); let wordr = len % U256::from(32); + let gas_ext_code = match &config.eip_2929 { + None => config.gas_ext_code, + Some(eip_2929) => if is_cold { + eip_2929.cold_account_access_cost + } else { + eip_2929.warm_storage_read_cost + }, + }; - let gas = U256::from(config.gas_ext_code).checked_add( + let gas = U256::from(gas_ext_code).checked_add( U256::from(G_COPY).checked_mul( if wordr == U256::zero() { wordd @@ -134,6 +142,28 @@ pub fn extcodecopy_cost(len: U256, config: &Config) -> Result { Ok(gas.as_u64()) } +pub fn account_access_cost(is_cold: bool, default_value: u64, config: &Config) -> u64 { + match &config.eip_2929 { + None => default_value, + Some(eip_2929) => if is_cold { + eip_2929.cold_account_access_cost + } else { + eip_2929.warm_storage_read_cost + }, + } +} + +pub fn storage_access_cost(is_cold: bool, default_value: u64, config: &Config) -> u64 { + match &config.eip_2929 { + None => default_value, + Some(eip_2929) => if is_cold { + eip_2929.cold_sload_cost + } else { + eip_2929.warm_storage_read_cost + }, + } +} + pub fn log_cost(n: u8, len: U256) -> Result { let gas = U256::from(G_LOG) .checked_add(U256::from(G_LOGDATA).checked_mul(len).ok_or(ExitError::OutOfGas)?) @@ -169,37 +199,54 @@ pub fn sha3_cost(len: U256) -> Result { Ok(gas.as_u64()) } -pub fn sstore_cost(original: H256, current: H256, new: H256, gas: u64, config: &Config) -> Result { - if config.sstore_gas_metering { +pub fn sstore_cost(original: H256, current: H256, new: H256, gas: u64, is_cold: bool, config: &Config) -> Result { + // modify costs if EIP-2929 is enabled + let (gas_sload, gas_sstore_reset) = match &config.eip_2929 { + None => (config.gas_sload, config.gas_sstore_reset), + Some(eip_2929) => (eip_2929.warm_storage_read_cost, config.gas_sstore_reset - eip_2929.cold_sload_cost), + }; + let gas_cost = if config.sstore_gas_metering { if config.sstore_revert_under_stipend { if gas < config.call_stipend { return Err(ExitError::OutOfGas) } } - Ok(if new == current { - config.gas_sload + if new == current { + gas_sload } else { if original == current { if original == H256::zero() { config.gas_sstore_set } else { - config.gas_sstore_reset + gas_sstore_reset } } else { - config.gas_sload + gas_sload } - }) + } } else { - Ok(if current == H256::zero() && new != H256::zero() { + if current == H256::zero() && new != H256::zero() { config.gas_sstore_set } else { - config.gas_sstore_reset - }) - } + gas_sstore_reset + } + }; + Ok( + // In EIP-2929 we charge extra if the slot has not been used yet in this transaction + if is_cold { + config + .eip_2929 + .as_ref() + .map(|eip2929| gas_cost + eip2929.cold_sload_cost) + .unwrap_or(gas_cost) + } else { + gas_cost + } + ) } -pub fn suicide_cost(value: U256, target_exists: bool, config: &Config) -> u64 { +pub fn suicide_cost(value: U256, is_cold: bool, target_exists: bool, config: &Config) -> u64 { let eip161 = !config.empty_considered_exists; let should_charge_topup = if eip161 { value != U256::zero() && !target_exists @@ -213,18 +260,36 @@ pub fn suicide_cost(value: U256, target_exists: bool, config: &Config) -> u64 { 0 }; - config.gas_suicide + suicide_gas_topup + let eip_2929_cost = match &config.eip_2929 { + None => 0, + Some(eip_2929) => if is_cold { + eip_2929.cold_account_access_cost + } else { + 0 + } + }; + + config.gas_suicide + suicide_gas_topup + eip_2929_cost } pub fn call_cost( value: U256, + is_cold: bool, is_call_or_callcode: bool, is_call_or_staticcall: bool, new_account: bool, config: &Config, ) -> u64 { let transfers_value = value != U256::default(); - config.gas_call + + let gas_call = match &config.eip_2929 { + None => config.gas_call, + Some(eip_2929) => if is_cold { + eip_2929.cold_account_access_cost + } else { + eip_2929.warm_storage_read_cost + } + }; + gas_call + xfer_cost(is_call_or_callcode, transfers_value) + new_cost(is_call_or_staticcall, new_account, transfers_value, config) } diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 790852055..f929c9124 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -5,6 +5,8 @@ #![cfg_attr(not(feature = "std"), no_std)] +extern crate alloc; + #[cfg(feature = "tracing")] pub mod tracing; @@ -26,6 +28,7 @@ mod costs; mod memory; mod utils; +use alloc::collections::BTreeSet; use core::cmp::max; use primitive_types::{H160, H256, U256}; use evm_core::{Opcode, ExitError, Stack}; @@ -70,6 +73,8 @@ impl<'config> Gasometer<'config> { used_gas: 0, refunded_gas: 0, config, + accessed_addresses: BTreeSet::new(), + accessed_storage_keys: BTreeSet::new(), }), } } @@ -77,11 +82,11 @@ impl<'config> Gasometer<'config> { #[inline] /// Returns the numerical gas cost value. pub fn gas_cost( - &self, + &mut self, cost: GasCost, gas: u64, ) -> Result { - match self.inner.as_ref() { + match self.inner.as_mut() { Ok(inner) => inner.gas_cost(cost, gas), Err(e) => Err(e.clone()) } @@ -233,6 +238,19 @@ impl<'config> Gasometer<'config> { Ok(()) } + /// Access an address (makes certain gas costs cheaper in the future) + pub fn access_address(&mut self, address: H160) -> Result<(), ExitError> { + let inner = self.inner_mut()?; + inner.accessed_addresses.insert(address); + Ok(()) + } + + pub fn access_storage(&mut self, address: H160, key: H256) -> Result<(), ExitError> { + let inner = self.inner_mut()?; + inner.accessed_storage_keys.insert((address, key)); + Ok(()) + } + /// Record transaction cost. pub fn record_transaction( &mut self, @@ -448,26 +466,41 @@ pub fn dynamic_opcode_cost( Opcode::SELFBALANCE if config.has_self_balance => GasCost::Low, Opcode::SELFBALANCE => GasCost::Invalid, - Opcode::EXTCODESIZE => GasCost::ExtCodeSize, - Opcode::BALANCE => GasCost::Balance, + Opcode::EXTCODESIZE => GasCost::ExtCodeSize { + address: stack.peek(0)?.into(), + }, + Opcode::BALANCE => GasCost::Balance { + address: stack.peek(0)?.into(), + }, Opcode::BLOCKHASH => GasCost::BlockHash, - Opcode::EXTCODEHASH if config.has_ext_code_hash => GasCost::ExtCodeHash, + Opcode::EXTCODEHASH if config.has_ext_code_hash => GasCost::ExtCodeHash { + address: stack.peek(0)?.into(), + }, Opcode::EXTCODEHASH => GasCost::Invalid, - Opcode::CALLCODE => GasCost::CallCode { - value: U256::from_big_endian(&stack.peek(2)?[..]), - gas: U256::from_big_endian(&stack.peek(0)?[..]), - target_exists: handler.exists(stack.peek(1)?.into()), - }, - Opcode::STATICCALL => GasCost::StaticCall { - gas: U256::from_big_endian(&stack.peek(0)?[..]), - target_exists: handler.exists(stack.peek(1)?.into()), - }, + Opcode::CALLCODE => { + let target = stack.peek(1)?.into(); + GasCost::CallCode { + value: U256::from_big_endian(&stack.peek(2)?[..]), + gas: U256::from_big_endian(&stack.peek(0)?[..]), + target, + target_exists: handler.exists(target), + } + } + Opcode::STATICCALL => { + let target = stack.peek(1)?.into(); + GasCost::StaticCall { + gas: U256::from_big_endian(&stack.peek(0)?[..]), + target, + target_exists: handler.exists(target), + } + } Opcode::SHA3 => GasCost::Sha3 { len: U256::from_big_endian(&stack.peek(1)?[..]), }, Opcode::EXTCODECOPY => GasCost::ExtCodeCopy { + address: stack.peek(0)?.into(), len: U256::from_big_endian(&stack.peek(3)?[..]), }, Opcode::CALLDATACOPY | Opcode::CODECOPY => GasCost::VeryLowCopy { @@ -476,12 +509,19 @@ pub fn dynamic_opcode_cost( Opcode::EXP => GasCost::Exp { power: U256::from_big_endian(&stack.peek(1)?[..]), }, - Opcode::SLOAD => GasCost::SLoad, - - Opcode::DELEGATECALL if config.has_delegate_call => GasCost::DelegateCall { - gas: U256::from_big_endian(&stack.peek(0)?[..]), - target_exists: handler.exists(stack.peek(1)?.into()), + Opcode::SLOAD => GasCost::SLoad { + address, + key: stack.peek(0)?, }, + + Opcode::DELEGATECALL if config.has_delegate_call => { + let target = stack.peek(1)?.into(); + GasCost::DelegateCall { + gas: U256::from_big_endian(&stack.peek(0)?[..]), + target, + target_exists: handler.exists(target), + } + } Opcode::DELEGATECALL => GasCost::Invalid, Opcode::RETURNDATASIZE if config.has_return_data => GasCost::Base, @@ -495,6 +535,8 @@ pub fn dynamic_opcode_cost( let value = stack.peek(1)?; GasCost::SStore { + address, + key: index, original: handler.original_storage(address, index), current: handler.storage(address, index), new: value, @@ -524,19 +566,26 @@ pub fn dynamic_opcode_cost( Opcode::CREATE2 if !is_static && config.has_create2 => GasCost::Create2 { len: U256::from_big_endian(&stack.peek(2)?[..]), }, - Opcode::SUICIDE if !is_static => GasCost::Suicide { - value: handler.balance(address), - target_exists: handler.exists(stack.peek(0)?.into()), - already_removed: handler.deleted(address), - }, + Opcode::SUICIDE if !is_static => { + let target = stack.peek(0)?.into(); + GasCost::Suicide { + value: handler.balance(address), + target, + target_exists: handler.exists(target), + already_removed: handler.deleted(address), + } + } Opcode::CALL if !is_static || - (is_static && U256::from_big_endian(&stack.peek(2)?[..]) == U256::zero()) => + (is_static && U256::from_big_endian(&stack.peek(2)?[..]) == U256::zero()) => { + let target = stack.peek(1)?.into(); GasCost::Call { value: U256::from_big_endian(&stack.peek(2)?[..]), gas: U256::from_big_endian(&stack.peek(0)?[..]), - target_exists: handler.exists(stack.peek(1)?.into()), - }, + target, + target_exists: handler.exists(target), + } + } _ => GasCost::Invalid, }; @@ -605,6 +654,8 @@ struct Inner<'config> { used_gas: u64, refunded_gas: i64, config: &'config Config, + accessed_addresses: BTreeSet, + accessed_storage_keys: BTreeSet<(H160, H256)>, } impl<'config> Inner<'config> { @@ -652,33 +703,35 @@ impl<'config> Inner<'config> { /// Returns the gas cost numerical value. fn gas_cost( - &self, + &mut self, cost: GasCost, gas: u64, ) -> Result { Ok(match cost { - GasCost::Call { value, target_exists, .. } => - costs::call_cost(value, true, true, !target_exists, self.config), - GasCost::CallCode { value, target_exists, .. } => - costs::call_cost(value, true, false, !target_exists, self.config), - GasCost::DelegateCall { target_exists, .. } => - costs::call_cost(U256::zero(), false, false, !target_exists, self.config), - GasCost::StaticCall { target_exists, .. } => - costs::call_cost(U256::zero(), false, true, !target_exists, self.config), - GasCost::Suicide { value, target_exists, .. } => - costs::suicide_cost(value, target_exists, self.config), + GasCost::Call { value, target, target_exists, .. } => + costs::call_cost(value, self.is_address_cold(target), true, true, !target_exists, self.config), + GasCost::CallCode { value, target, target_exists, .. } => + costs::call_cost(value, self.is_address_cold(target), true, false, !target_exists, self.config), + GasCost::DelegateCall { target, target_exists, .. } => + costs::call_cost(U256::zero(), self.is_address_cold(target), false, false, !target_exists, self.config), + GasCost::StaticCall { target, target_exists, .. } => + costs::call_cost(U256::zero(), self.is_address_cold(target), false, true, !target_exists, self.config), + GasCost::Suicide { value, target, target_exists, .. } => + costs::suicide_cost(value, self.is_address_cold(target), target_exists, self.config), GasCost::SStore { .. } if self.config.estimate => self.config.gas_sstore_set, - GasCost::SStore { original, current, new } => - costs::sstore_cost(original, current, new, gas, self.config)?, + GasCost::SStore { address, key, original, current, new } => + costs::sstore_cost(original, current, new, gas, self.is_storage_cold(address, key), self.config)?, GasCost::Sha3 { len } => costs::sha3_cost(len)?, GasCost::Log { n, len } => costs::log_cost(n, len)?, - GasCost::ExtCodeCopy { len } => costs::extcodecopy_cost(len, self.config)?, + GasCost::ExtCodeCopy { address, len } => + costs::extcodecopy_cost(len, self.is_address_cold(address), self.config)?, GasCost::VeryLowCopy { len } => costs::verylowcopy_cost(len)?, GasCost::Exp { power } => costs::exp_cost(power, self.config)?, GasCost::Create => consts::G_CREATE, GasCost::Create2 { len } => costs::create2_cost(len)?, - GasCost::SLoad => self.config.gas_sload, + GasCost::SLoad { address, key } => + costs::storage_access_cost(self.is_storage_cold(address, key), self.config.gas_sload, self.config), GasCost::Zero => consts::G_ZERO, GasCost::Base => consts::G_BASE, @@ -686,13 +739,33 @@ impl<'config> Inner<'config> { GasCost::Low => consts::G_LOW, GasCost::Invalid => return Err(ExitError::OutOfGas), - GasCost::ExtCodeSize => self.config.gas_ext_code, - GasCost::Balance => self.config.gas_balance, + GasCost::ExtCodeSize { address } => + costs::account_access_cost(self.is_address_cold(address), self.config.gas_ext_code, self.config), + GasCost::Balance { address } => + costs::account_access_cost(self.is_address_cold(address), self.config.gas_balance, self.config), GasCost::BlockHash => consts::G_BLOCKHASH, - GasCost::ExtCodeHash => self.config.gas_ext_code_hash, + GasCost::ExtCodeHash { address } => + costs::account_access_cost(self.is_address_cold(address), self.config.gas_ext_code_hash, self.config), }) } + fn is_address_cold(&mut self, address: H160) -> bool { + let is_cold = !self.accessed_addresses.contains(&address); + if is_cold { + self.accessed_addresses.insert(address); + } + is_cold + } + + fn is_storage_cold(&mut self, address: H160, key: H256) -> bool { + let tuple = (address, key); + let is_cold = !self.accessed_storage_keys.contains(&tuple); + if is_cold { + self.accessed_storage_keys.insert(tuple); + } + is_cold + } + fn gas_refund( &self, cost: GasCost @@ -700,7 +773,7 @@ impl<'config> Inner<'config> { match cost { _ if self.config.estimate => 0, - GasCost::SStore { original, current, new } => + GasCost::SStore { original, current, new, .. } => costs::sstore_refund(original, current, new, self.config), GasCost::Suicide { already_removed, .. } => costs::suicide_refund(already_removed), @@ -724,13 +797,22 @@ pub enum GasCost { Invalid, /// Gas cost for `EXTCODESIZE`. - ExtCodeSize, + ExtCodeSize { + /// Address to get code size of + address: H160, + }, /// Gas cost for `BALANCE`. - Balance, + Balance { + /// Address to get balance of + address: H160, + }, /// Gas cost for `BLOCKHASH`. BlockHash, /// Gas cost for `EXTBLOCKHASH`. - ExtCodeHash, + ExtCodeHash { + /// Address to get code hash of + address: H160, + }, /// Gas cost for `CALL`. Call { @@ -738,6 +820,8 @@ pub enum GasCost { value: U256, /// Call gas. gas: U256, + /// Address to call + target: H160, /// Whether the target exists. target_exists: bool }, @@ -747,6 +831,8 @@ pub enum GasCost { value: U256, /// Call gas. gas: U256, + /// Address to call + target: H160, /// Whether the target exists. target_exists: bool }, @@ -754,6 +840,8 @@ pub enum GasCost { DelegateCall { /// Call gas. gas: U256, + /// Address to call + target: H160, /// Whether the target exists. target_exists: bool }, @@ -761,6 +849,8 @@ pub enum GasCost { StaticCall { /// Call gas. gas: U256, + /// Address to call + target: H160, /// Whether the target exists. target_exists: bool }, @@ -768,6 +858,8 @@ pub enum GasCost { Suicide { /// Value. value: U256, + /// ETH recipient + target: H160, /// Whether the target exists. target_exists: bool, /// Whether the target has already been removed. @@ -775,6 +867,10 @@ pub enum GasCost { }, /// Gas cost for `SSTORE`. SStore { + /// Address the storage is for + address: H160, + /// Key + key: H256, /// Original value. original: H256, /// Current value. @@ -796,6 +892,8 @@ pub enum GasCost { }, /// Gas cost for `EXTCODECOPY`. ExtCodeCopy { + /// Address to copy code from + address: H160, /// Length. len: U256 }, @@ -817,7 +915,12 @@ pub enum GasCost { len: U256 }, /// Gas cost for `SLOAD`. - SLoad, + SLoad { + /// Address the storage is for + address: H160, + /// Key to read + key: H256, + }, } /// Memory cost. diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 09e355f29..637bc9858 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -161,6 +161,14 @@ impl<'config> Runtime<'config> { } } +/// Parameters specific to EIP-2929 (see https://eips.ethereum.org/EIPS/eip-2929) +#[derive(Clone, Debug)] +pub struct EIP2929Config { + pub cold_sload_cost: u64, + pub cold_account_access_cost: u64, + pub warm_storage_read_cost: u64, +} + /// Runtime configuration. #[derive(Clone, Debug)] pub struct Config { @@ -236,6 +244,7 @@ pub struct Config { pub has_ext_code_hash: bool, /// Whether the gasometer is running in estimate mode. pub estimate: bool, + pub eip_2929: Option, } impl Config { @@ -277,6 +286,7 @@ impl Config { has_self_balance: false, has_ext_code_hash: false, estimate: false, + eip_2929: None, } } @@ -318,6 +328,53 @@ impl Config { has_self_balance: true, has_ext_code_hash: true, estimate: false, + eip_2929: None, + } + } + + /// Berlin hard fork configuration. + pub const fn berlin() -> Config { + Config { + gas_ext_code: 700, + gas_ext_code_hash: 700, + gas_balance: 700, + gas_sload: 800, + gas_sstore_set: 20000, + gas_sstore_reset: 5000, + refund_sstore_clears: 15000, + gas_suicide: 5000, + gas_suicide_new_account: 25000, + gas_call: 700, + gas_expbyte: 50, + gas_transaction_create: 53000, + gas_transaction_call: 21000, + gas_transaction_zero_data: 4, + gas_transaction_non_zero_data: 16, + sstore_gas_metering: true, + sstore_revert_under_stipend: true, + err_on_call_with_more_gas: false, + empty_considered_exists: false, + create_increase_nonce: true, + call_l64_after_gas: true, + stack_limit: 1024, + memory_limit: usize::max_value(), + call_stack_limit: 1024, + create_contract_limit: Some(0x6000), + call_stipend: 2300, + has_delegate_call: true, + has_create2: true, + has_revert: true, + has_return_data: true, + has_bitwise_shifting: true, + has_chain_id: true, + has_self_balance: true, + has_ext_code_hash: true, + estimate: false, + eip_2929: Some(EIP2929Config { + cold_sload_cost: 2100, + cold_account_access_cost: 2600, + warm_storage_read_cost: 100, + }), } } } diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 522ad80b9..7bf946abc 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -245,6 +245,17 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { } } + // TODO: Need to add the precompile addresses too (see EIP-2929 spec) + fn init_access_addresses( + gasometer: &mut Gasometer, + caller: H160, + address: H160 + ) -> Result<(), ExitError> { + gasometer.access_address(caller)?; + gasometer.access_address(address)?; + Ok(()) + } + /// Execute a `CALL` transaction. pub fn transact_call( &mut self, @@ -255,11 +266,18 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { gas_limit: u64, ) -> (ExitReason, Vec) { let transaction_cost = gasometer::call_transaction_cost(&data); - match self.state.metadata_mut().gasometer.record_transaction(transaction_cost) { - Ok(()) => (), - Err(e) => return (e.into(), Vec::new()), + { + let gasometer = &mut self.state.metadata_mut().gasometer; + match gasometer + .record_transaction(transaction_cost) + .and(Self::init_access_addresses(gasometer, caller, address)) + { + Ok(()) => (), + Err(e) => return (e.into(), Vec::new()), + } } + self.state.inc_nonce(caller); let context = Context { @@ -349,6 +367,10 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { let address = self.create_address(scheme); + try_or_fail!( + Self::init_access_addresses(&mut self.state.metadata_mut().gasometer, caller, address) + ); + event!(Create { caller, address, From 5ee05aedfd890892d0ee4209aaf1607a4408831a Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Thu, 1 Jul 2021 16:45:32 +0700 Subject: [PATCH 04/38] Cleanup --- gasometer/src/lib.rs | 50 +++++++++++++++++++++++---------------- runtime/src/lib.rs | 38 ++++++++++++++++------------- src/executor/stack/mod.rs | 2 +- 3 files changed, 52 insertions(+), 38 deletions(-) diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index f929c9124..a4adca8d1 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -65,6 +65,12 @@ pub struct Gasometer<'config> { impl<'config> Gasometer<'config> { /// Create a new gasometer with given gas limit and config. pub fn new(gas_limit: u64, config: &'config Config) -> Self { + let (accessed_addresses, accessed_storage_keys) = if config.increase_state_access_gas { + (Some(BTreeSet::new()), Some(BTreeSet::new())) + } else { + (None, None) + }; + Self { gas_limit, config, @@ -73,8 +79,8 @@ impl<'config> Gasometer<'config> { used_gas: 0, refunded_gas: 0, config, - accessed_addresses: BTreeSet::new(), - accessed_storage_keys: BTreeSet::new(), + accessed_addresses, + accessed_storage_keys, }), } } @@ -82,11 +88,11 @@ impl<'config> Gasometer<'config> { #[inline] /// Returns the numerical gas cost value. pub fn gas_cost( - &mut self, + &self, cost: GasCost, gas: u64, ) -> Result { - match self.inner.as_mut() { + match self.inner.as_ref() { Ok(inner) => inner.gas_cost(cost, gas), Err(e) => Err(e.clone()) } @@ -241,13 +247,17 @@ impl<'config> Gasometer<'config> { /// Access an address (makes certain gas costs cheaper in the future) pub fn access_address(&mut self, address: H160) -> Result<(), ExitError> { let inner = self.inner_mut()?; - inner.accessed_addresses.insert(address); + if let Some(accessed_addresses) = &mut inner.accessed_addresses { + accessed_addresses.insert(address); + } Ok(()) } pub fn access_storage(&mut self, address: H160, key: H256) -> Result<(), ExitError> { let inner = self.inner_mut()?; - inner.accessed_storage_keys.insert((address, key)); + if let Some(accessed_storage_keys) = &mut inner.accessed_storage_keys { + accessed_storage_keys.insert((address, key)); + } Ok(()) } @@ -654,8 +664,8 @@ struct Inner<'config> { used_gas: u64, refunded_gas: i64, config: &'config Config, - accessed_addresses: BTreeSet, - accessed_storage_keys: BTreeSet<(H160, H256)>, + accessed_addresses: Option>, + accessed_storage_keys: Option>, } impl<'config> Inner<'config> { @@ -703,7 +713,7 @@ impl<'config> Inner<'config> { /// Returns the gas cost numerical value. fn gas_cost( - &mut self, + &self, cost: GasCost, gas: u64, ) -> Result { @@ -749,21 +759,21 @@ impl<'config> Inner<'config> { }) } - fn is_address_cold(&mut self, address: H160) -> bool { - let is_cold = !self.accessed_addresses.contains(&address); - if is_cold { - self.accessed_addresses.insert(address); + fn is_address_cold(&self, address: H160) -> bool { + if let Some(accessed_addresses) = &self.accessed_addresses { + !accessed_addresses.contains(&address) + } else { + false } - is_cold } - fn is_storage_cold(&mut self, address: H160, key: H256) -> bool { - let tuple = (address, key); - let is_cold = !self.accessed_storage_keys.contains(&tuple); - if is_cold { - self.accessed_storage_keys.insert(tuple); + fn is_storage_cold(&self, address: H160, key: H256) -> bool { + if let Some(accessed_storage_keys) = &self.accessed_storage_keys { + let tuple = (address, key); + !accessed_storage_keys.contains(&tuple) + } else { + false } - is_cold } fn gas_refund( diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 637bc9858..60b6327aa 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -161,14 +161,6 @@ impl<'config> Runtime<'config> { } } -/// Parameters specific to EIP-2929 (see https://eips.ethereum.org/EIPS/eip-2929) -#[derive(Clone, Debug)] -pub struct EIP2929Config { - pub cold_sload_cost: u64, - pub cold_account_access_cost: u64, - pub warm_storage_read_cost: u64, -} - /// Runtime configuration. #[derive(Clone, Debug)] pub struct Config { @@ -186,6 +178,8 @@ pub struct Config { pub gas_balance: u64, /// Gas paid for SLOAD opcode. pub gas_sload: u64, + /// Gas paid for cold SLOAD opcode. + pub gas_sload_cold: u64, /// Gas paid for SUICIDE opcode. pub gas_suicide: u64, /// Gas paid for SUICIDE opcode when it hits a new account. @@ -202,10 +196,16 @@ pub struct Config { pub gas_transaction_zero_data: u64, /// Gas paid for non-zero data in a transaction. pub gas_transaction_non_zero_data: u64, + /// Gas paid for accessing cold account. + pub gas_account_access_cold: u64, + /// Gas paid for accessing ready storage. + pub gas_storage_read_warm: u64, /// EIP-1283. pub sstore_gas_metering: bool, /// EIP-1706. pub sstore_revert_under_stipend: bool, + /// EIP-2929 + pub increase_state_access_gas: bool, /// Whether to throw out of gas error when /// CALL/CALLCODE/DELEGATECALL requires more than maximum amount /// of gas. @@ -244,7 +244,6 @@ pub struct Config { pub has_ext_code_hash: bool, /// Whether the gasometer is running in estimate mode. pub estimate: bool, - pub eip_2929: Option, } impl Config { @@ -255,6 +254,7 @@ impl Config { gas_ext_code_hash: 20, gas_balance: 20, gas_sload: 50, + gas_sload_cold: 0, gas_sstore_set: 20000, gas_sstore_reset: 5000, refund_sstore_clears: 15000, @@ -266,8 +266,11 @@ impl Config { gas_transaction_call: 21000, gas_transaction_zero_data: 4, gas_transaction_non_zero_data: 68, + gas_account_access_cold: 0, + gas_storage_read_warm: 0, sstore_gas_metering: false, sstore_revert_under_stipend: false, + increase_state_access_gas: false, err_on_call_with_more_gas: true, empty_considered_exists: true, create_increase_nonce: false, @@ -286,7 +289,6 @@ impl Config { has_self_balance: false, has_ext_code_hash: false, estimate: false, - eip_2929: None, } } @@ -297,6 +299,7 @@ impl Config { gas_ext_code_hash: 700, gas_balance: 700, gas_sload: 800, + gas_sload_cold: 0, gas_sstore_set: 20000, gas_sstore_reset: 5000, refund_sstore_clears: 15000, @@ -308,8 +311,11 @@ impl Config { gas_transaction_call: 21000, gas_transaction_zero_data: 4, gas_transaction_non_zero_data: 16, + gas_account_access_cold: 0, + gas_storage_read_warm: 0, sstore_gas_metering: true, sstore_revert_under_stipend: true, + increase_state_access_gas: false, err_on_call_with_more_gas: false, empty_considered_exists: false, create_increase_nonce: true, @@ -328,7 +334,6 @@ impl Config { has_self_balance: true, has_ext_code_hash: true, estimate: false, - eip_2929: None, } } @@ -339,6 +344,7 @@ impl Config { gas_ext_code_hash: 700, gas_balance: 700, gas_sload: 800, + gas_sload_cold: 2100, gas_sstore_set: 20000, gas_sstore_reset: 5000, refund_sstore_clears: 15000, @@ -350,14 +356,17 @@ impl Config { gas_transaction_call: 21000, gas_transaction_zero_data: 4, gas_transaction_non_zero_data: 16, + gas_account_access_cold: 2600, + gas_storage_read_warm: 100, sstore_gas_metering: true, sstore_revert_under_stipend: true, + increase_state_access_gas: false, err_on_call_with_more_gas: false, empty_considered_exists: false, create_increase_nonce: true, call_l64_after_gas: true, stack_limit: 1024, - memory_limit: usize::max_value(), + memory_limit: usize::MAX, call_stack_limit: 1024, create_contract_limit: Some(0x6000), call_stipend: 2300, @@ -370,11 +379,6 @@ impl Config { has_self_balance: true, has_ext_code_hash: true, estimate: false, - eip_2929: Some(EIP2929Config { - cold_sload_cost: 2100, - cold_account_access_cost: 2600, - warm_storage_read_cost: 100, - }), } } } diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 7bf946abc..9186ec0bf 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -270,7 +270,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { let gasometer = &mut self.state.metadata_mut().gasometer; match gasometer .record_transaction(transaction_cost) - .and(Self::init_access_addresses(gasometer, caller, address)) + .and_then(Self::init_access_addresses(gasometer, caller, address)) { Ok(()) => (), Err(e) => return (e.into(), Vec::new()), From 7654641a5f726118e62fac7968f98211f5ca673c Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Thu, 1 Jul 2021 17:38:40 +0700 Subject: [PATCH 05/38] Clean up costs --- gasometer/src/costs.rs | 55 +++++++++++------------------------------- gasometer/src/lib.rs | 11 +++++---- runtime/src/lib.rs | 11 +++++---- 3 files changed, 26 insertions(+), 51 deletions(-) diff --git a/gasometer/src/costs.rs b/gasometer/src/costs.rs index 09a25cc4f..5e0a55386 100644 --- a/gasometer/src/costs.rs +++ b/gasometer/src/costs.rs @@ -116,16 +116,7 @@ pub fn verylowcopy_cost(len: U256) -> Result { pub fn extcodecopy_cost(len: U256, is_cold: bool, config: &Config) -> Result { let wordd = len / U256::from(32); let wordr = len % U256::from(32); - let gas_ext_code = match &config.eip_2929 { - None => config.gas_ext_code, - Some(eip_2929) => if is_cold { - eip_2929.cold_account_access_cost - } else { - eip_2929.warm_storage_read_cost - }, - }; - - let gas = U256::from(gas_ext_code).checked_add( + let gas = U256::from(storage_access_cost(is_cold, config.gas_ext_code, config)).checked_add( U256::from(G_COPY).checked_mul( if wordr == U256::zero() { wordd @@ -142,28 +133,6 @@ pub fn extcodecopy_cost(len: U256, is_cold: bool, config: &Config) -> Result u64 { - match &config.eip_2929 { - None => default_value, - Some(eip_2929) => if is_cold { - eip_2929.cold_account_access_cost - } else { - eip_2929.warm_storage_read_cost - }, - } -} - -pub fn storage_access_cost(is_cold: bool, default_value: u64, config: &Config) -> u64 { - match &config.eip_2929 { - None => default_value, - Some(eip_2929) => if is_cold { - eip_2929.cold_sload_cost - } else { - eip_2929.warm_storage_read_cost - }, - } -} - pub fn log_cost(n: u8, len: U256) -> Result { let gas = U256::from(G_LOG) .checked_add(U256::from(G_LOGDATA).checked_mul(len).ok_or(ExitError::OutOfGas)?) @@ -281,19 +250,23 @@ pub fn call_cost( config: &Config, ) -> u64 { let transfers_value = value != U256::default(); - let gas_call = match &config.eip_2929 { - None => config.gas_call, - Some(eip_2929) => if is_cold { - eip_2929.cold_account_access_cost - } else { - eip_2929.warm_storage_read_cost - } - }; - gas_call + + storage_access_cost(is_cold, config.gas_call, config) + xfer_cost(is_call_or_callcode, transfers_value) + new_cost(is_call_or_staticcall, new_account, transfers_value, config) } +pub fn storage_access_cost(is_cold: bool, regular_value: u64, config: &Config) -> u64 { + if config.increase_state_access_gas { + if is_cold { + config.gas_account_access_cold + } else { + config.gas_storage_read_warm + } + } else { + regular_value + } +} + fn xfer_cost( is_call_or_callcode: bool, transfers_value: bool diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index a4adca8d1..1b57f62f6 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -726,6 +726,7 @@ impl<'config> Inner<'config> { costs::call_cost(U256::zero(), self.is_address_cold(target), false, false, !target_exists, self.config), GasCost::StaticCall { target, target_exists, .. } => costs::call_cost(U256::zero(), self.is_address_cold(target), false, true, !target_exists, self.config), + GasCost::Suicide { value, target, target_exists, .. } => costs::suicide_cost(value, self.is_address_cold(target), target_exists, self.config), GasCost::SStore { .. } if self.config.estimate => self.config.gas_sstore_set, @@ -734,8 +735,6 @@ impl<'config> Inner<'config> { GasCost::Sha3 { len } => costs::sha3_cost(len)?, GasCost::Log { n, len } => costs::log_cost(n, len)?, - GasCost::ExtCodeCopy { address, len } => - costs::extcodecopy_cost(len, self.is_address_cold(address), self.config)?, GasCost::VeryLowCopy { len } => costs::verylowcopy_cost(len)?, GasCost::Exp { power } => costs::exp_cost(power, self.config)?, GasCost::Create => consts::G_CREATE, @@ -750,12 +749,14 @@ impl<'config> Inner<'config> { GasCost::Invalid => return Err(ExitError::OutOfGas), GasCost::ExtCodeSize { address } => - costs::account_access_cost(self.is_address_cold(address), self.config.gas_ext_code, self.config), + costs::storage_access_cost(self.is_address_cold(address), self.config.gas_ext_code, self.config), + GasCost::ExtCodeCopy { address, len } => + costs::extcodecopy_cost(len, self.is_address_cold(address), self.config)?, GasCost::Balance { address } => - costs::account_access_cost(self.is_address_cold(address), self.config.gas_balance, self.config), + costs::storage_access_cost(self.is_address_cold(address), self.config.gas_balance, self.config), GasCost::BlockHash => consts::G_BLOCKHASH, GasCost::ExtCodeHash { address } => - costs::account_access_cost(self.is_address_cold(address), self.config.gas_ext_code_hash, self.config), + costs::storage_access_cost(self.is_address_cold(address), self.config.gas_ext_code_hash, self.config), }) } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 60b6327aa..12b2bb99c 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -36,6 +36,7 @@ pub use crate::handler::{Transfer, Handler}; use alloc::vec::Vec; use alloc::rc::Rc; +use primitive_types::U256; macro_rules! step { ( $self:expr, $handler:expr, $return:tt $($err:path)?; $($ok:path)? ) => ({ @@ -340,17 +341,17 @@ impl Config { /// Berlin hard fork configuration. pub const fn berlin() -> Config { Config { - gas_ext_code: 700, - gas_ext_code_hash: 700, - gas_balance: 700, - gas_sload: 800, + gas_ext_code: 0, + gas_ext_code_hash: 0, + gas_balance: 0, + gas_sload: 0, gas_sload_cold: 2100, gas_sstore_set: 20000, gas_sstore_reset: 5000, refund_sstore_clears: 15000, gas_suicide: 5000, gas_suicide_new_account: 25000, - gas_call: 700, + gas_call: 0, gas_expbyte: 50, gas_transaction_create: 53000, gas_transaction_call: 21000, From a08f255bd35f2b11aae61380d77fc0bc73b8fce0 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Fri, 2 Jul 2021 15:22:34 +0700 Subject: [PATCH 06/38] Add `Precompiles` trait --- src/executor/stack/mod.rs | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 9186ec0bf..e23e11529 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -88,18 +88,19 @@ pub struct PrecompileOutput { pub logs: Vec, } -/// Precompiles function signature. Expected input arguments are: -/// * Address -/// * Input -/// * Context -/// * State -/// * Is static -type PrecompileFn = fn(H160, &[u8], Option, &Context, &mut S, bool) -> Option>; +/// Precompiles trait which allows for the execution of a precompile. +pub trait Precompiles { + /// Runs the precompile at the given address with input and context. + fn run(address: H160, input: &[u8], context: &Context, state: &mut S, is_static: bool) -> Option>; + + /// Returns the set of precompile addresses. + fn addresses(&self) -> &[H160]; +} /// Stack-based executor. -pub struct StackExecutor<'config, S> { +pub struct StackExecutor<'config, S: State, P: Precompiles> { config: &'config Config, - precompile: PrecompileFn, + precompiles: P, state: S, } @@ -114,7 +115,7 @@ fn no_precompile( None } -impl<'config, S: StackState<'config>> StackExecutor<'config, S> { +impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, P> { /// Create a new stack-based executor. pub fn new( state: S, @@ -138,7 +139,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { ) -> Self { Self { config, - precompile, + precompiles: precompile, state, } } @@ -245,14 +246,17 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { } } - // TODO: Need to add the precompile addresses too (see EIP-2929 spec) fn init_access_addresses( + &self, gasometer: &mut Gasometer, caller: H160, address: H160 ) -> Result<(), ExitError> { gasometer.access_address(caller)?; gasometer.access_address(address)?; + for address in self.precompiles.addresses() { + gasometer.access_address(*address)?; + } Ok(()) } @@ -270,14 +274,13 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { let gasometer = &mut self.state.metadata_mut().gasometer; match gasometer .record_transaction(transaction_cost) - .and_then(Self::init_access_addresses(gasometer, caller, address)) + .and_then(self.init_access_addresses(gasometer, caller, address)) { Ok(()) => (), Err(e) => return (e.into(), Vec::new()), } } - self.state.inc_nonce(caller); let context = Context { @@ -368,7 +371,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { let address = self.create_address(scheme); try_or_fail!( - Self::init_access_addresses(&mut self.state.metadata_mut().gasometer, caller, address) + self.state.metadata_mut().gasometer.access_address(address) ); event!(Create { @@ -583,7 +586,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { } } - if let Some(ret) = (self.precompile)(code_address, &input, Some(gas_limit), &context, &mut self.state, is_static) { + if let Some(ret) = (self.precompiles)(code_address, &input, Some(gas_limit), &context, &mut self.state, is_static) { match ret { Ok(PrecompileOutput { exit_status , output, cost, logs }) => { for Log { address, topics, data} in logs { @@ -638,7 +641,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { } } -impl<'config, S: StackState<'config>> Handler for StackExecutor<'config, S> { +impl<'config, S: StackState<'config>, P: Precompiles> Handler for StackExecutor<'config, S, P> { type CreateInterrupt = Infallible; type CreateFeedback = Infallible; type CallInterrupt = Infallible; From 4d7c084ff96fe09aecacddc2099d152a1265a64b Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Fri, 2 Jul 2021 15:32:16 +0700 Subject: [PATCH 07/38] Update other costs --- gasometer/src/costs.rs | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/gasometer/src/costs.rs b/gasometer/src/costs.rs index 5e0a55386..8189cffd8 100644 --- a/gasometer/src/costs.rs +++ b/gasometer/src/costs.rs @@ -169,10 +169,10 @@ pub fn sha3_cost(len: U256) -> Result { } pub fn sstore_cost(original: H256, current: H256, new: H256, gas: u64, is_cold: bool, config: &Config) -> Result { - // modify costs if EIP-2929 is enabled - let (gas_sload, gas_sstore_reset) = match &config.eip_2929 { - None => (config.gas_sload, config.gas_sstore_reset), - Some(eip_2929) => (eip_2929.warm_storage_read_cost, config.gas_sstore_reset - eip_2929.cold_sload_cost), + let (gas_sload, gas_sstore_reset) = if config.increase_state_access_gas { + (config.gas_storage_read_warm, config.gas_sstore_reset - config.gas_sload_cold) + } else { + (config.gas_sload, config.gas_sstore_reset) }; let gas_cost = if config.sstore_gas_metering { if config.sstore_revert_under_stipend { @@ -204,11 +204,7 @@ pub fn sstore_cost(original: H256, current: H256, new: H256, gas: u64, is_cold: Ok( // In EIP-2929 we charge extra if the slot has not been used yet in this transaction if is_cold { - config - .eip_2929 - .as_ref() - .map(|eip2929| gas_cost + eip2929.cold_sload_cost) - .unwrap_or(gas_cost) + gas_cost + config.gas_sload_cold } else { gas_cost } @@ -229,16 +225,11 @@ pub fn suicide_cost(value: U256, is_cold: bool, target_exists: bool, config: &Co 0 }; - let eip_2929_cost = match &config.eip_2929 { - None => 0, - Some(eip_2929) => if is_cold { - eip_2929.cold_account_access_cost - } else { - 0 - } - }; - - config.gas_suicide + suicide_gas_topup + eip_2929_cost + let mut gas = config.gas_suicide + suicide_gas_topup; + if config.increase_state_access_gas && is_cold { + gas += config.gas_account_access_cold + } + gas } pub fn call_cost( From 770515127ce8c99da636560a65a9af0238fba18a Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Fri, 2 Jul 2021 17:51:52 +0700 Subject: [PATCH 08/38] General fixes --- runtime/src/lib.rs | 1 - src/executor/stack/mod.rs | 103 +++++++++++++++++++++----------------- 2 files changed, 57 insertions(+), 47 deletions(-) diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 12b2bb99c..f81fee606 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -36,7 +36,6 @@ pub use crate::handler::{Transfer, Handler}; use alloc::vec::Vec; use alloc::rc::Rc; -use primitive_types::U256; macro_rules! step { ( $self:expr, $handler:expr, $return:tt $($err:path)?; $($ok:path)? ) => ({ diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index e23e11529..da3b5edaa 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -89,41 +89,55 @@ pub struct PrecompileOutput { } /// Precompiles trait which allows for the execution of a precompile. -pub trait Precompiles { +pub trait Precompiles { /// Runs the precompile at the given address with input and context. - fn run(address: H160, input: &[u8], context: &Context, state: &mut S, is_static: bool) -> Option>; + fn run<'config, S: StackState<'config>>( + &self, + address: H160, + input: &[u8], + gas_limit: Option, + context: &Context, + state: &mut S, + is_static: bool, + ) -> Option>; /// Returns the set of precompile addresses. fn addresses(&self) -> &[H160]; } +#[derive(Default)] +struct NoPrecompile { + addresses: Vec, +} + +impl Precompiles for NoPrecompile { + fn run<'config, S: StackState<'config>>(&self, _address: H160, _input: &[u8], _gas_limit: Option,_context: &Context, _state: &mut S, _is_static: bool) -> Option> { + None + } + + fn addresses(&self) -> &[H160] { + &self.addresses + } +} + /// Stack-based executor. -pub struct StackExecutor<'config, S: State, P: Precompiles> { +pub struct StackExecutor<'config, S: StackState<'config>, P: Precompiles> { config: &'config Config, precompiles: P, state: S, } -fn no_precompile( - _address: H160, - _input: &[u8], - _target_gas: Option, - _context: &Context, - _state: &mut S, - _is_static: bool, -) -> Option> { - None -} - -impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, P> { +impl<'config, S: StackState<'config>> StackExecutor<'config, S, NoPrecompile> { /// Create a new stack-based executor. pub fn new( state: S, config: &'config Config, ) -> Self { - Self::new_with_precompile(state, config, no_precompile) + Self::new_with_precompile(state, config, NoPrecompile::default()) } +} +impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, P> { /// Return a reference of the Config. pub fn config( &self @@ -135,7 +149,7 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, pub fn new_with_precompile( state: S, config: &'config Config, - precompile: PrecompileFn, + precompile: P, ) -> Self { Self { config, @@ -246,20 +260,6 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, } } - fn init_access_addresses( - &self, - gasometer: &mut Gasometer, - caller: H160, - address: H160 - ) -> Result<(), ExitError> { - gasometer.access_address(caller)?; - gasometer.access_address(address)?; - for address in self.precompiles.addresses() { - gasometer.access_address(*address)?; - } - Ok(()) - } - /// Execute a `CALL` transaction. pub fn transact_call( &mut self, @@ -270,14 +270,25 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, gas_limit: u64, ) -> (ExitReason, Vec) { let transaction_cost = gasometer::call_transaction_cost(&data); - { - let gasometer = &mut self.state.metadata_mut().gasometer; - match gasometer - .record_transaction(transaction_cost) - .and_then(self.init_access_addresses(gasometer, caller, address)) - { - Ok(()) => (), - Err(e) => return (e.into(), Vec::new()), + + let gasometer = &mut self.state.metadata_mut().gasometer; + match gasometer + .record_transaction(transaction_cost) { + Ok(()) => (), + Err(e) => return (e.into(), Vec::new()), + } + + // Initialize initial addresses for EIP-2929 + if self.config.increase_state_access_gas { + let mut addresses: Vec = Vec::with_capacity( 2 + self.precompiles.addresses().len()); + addresses.push(caller); + addresses.push(address); + addresses.extend_from_slice(self.precompiles.addresses()); + for addr in self.precompiles.addresses() { + match gasometer.access_address(*addr) { + Ok(_) => {}, + Err(e) => return (e.into(), Vec::new()), + } } } @@ -586,10 +597,10 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, } } - if let Some(ret) = (self.precompiles)(code_address, &input, Some(gas_limit), &context, &mut self.state, is_static) { - match ret { - Ok(PrecompileOutput { exit_status , output, cost, logs }) => { - for Log { address, topics, data} in logs { + if let Some(ret) = self.precompiles.run(code_address, &input, Some(gas_limit), &context, &mut self.state, is_static) { + return match ret { + Ok(PrecompileOutput { exit_status, output, cost, logs }) => { + for Log { address, topics, data } in logs { match self.log(address, topics, data) { Ok(_) => continue, Err(error) => { @@ -600,11 +611,11 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, let _ = self.state.metadata_mut().gasometer.record_cost(cost); let _ = self.exit_substate(StackExitKind::Succeeded); - return Capture::Exit((ExitReason::Succeed(exit_status), output)); + Capture::Exit((ExitReason::Succeed(exit_status), output)) }, Err(e) => { let _ = self.exit_substate(StackExitKind::Failed); - return Capture::Exit((ExitReason::Error(e), Vec::new())); + Capture::Exit((ExitReason::Error(e), Vec::new())) }, } } @@ -641,7 +652,7 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, } } -impl<'config, S: StackState<'config>, P: Precompiles> Handler for StackExecutor<'config, S, P> { +impl<'config, S: StackState<'config>, P: Precompiles> Handler for StackExecutor<'config, S, P> { type CreateInterrupt = Infallible; type CreateFeedback = Infallible; type CallInterrupt = Infallible; From 238f26220bd3ab32528083eda1ff8e93b286ec41 Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Mon, 5 Jul 2021 10:10:39 -0400 Subject: [PATCH 09/38] Avoid unnecessary Vec allocation --- gasometer/src/lib.rs | 13 +++++++++++++ src/executor/stack/mod.rs | 18 +++++++++--------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 1b57f62f6..1b7d442c6 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -253,6 +253,19 @@ impl<'config> Gasometer<'config> { Ok(()) } + pub fn access_addresses(&mut self, addresses: I) -> Result<(), ExitError> + where + I: Iterator + { + let inner = self.inner_mut()?; + if let Some(accessed_addresses) = &mut inner.accessed_addresses { + for address in addresses { + accessed_addresses.insert(address); + } + } + Ok(()) + } + pub fn access_storage(&mut self, address: H160, key: H256) -> Result<(), ExitError> { let inner = self.inner_mut()?; if let Some(accessed_storage_keys) = &mut inner.accessed_storage_keys { diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index da3b5edaa..3427c5438 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -280,15 +280,15 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, // Initialize initial addresses for EIP-2929 if self.config.increase_state_access_gas { - let mut addresses: Vec = Vec::with_capacity( 2 + self.precompiles.addresses().len()); - addresses.push(caller); - addresses.push(address); - addresses.extend_from_slice(self.precompiles.addresses()); - for addr in self.precompiles.addresses() { - match gasometer.access_address(*addr) { - Ok(_) => {}, - Err(e) => return (e.into(), Vec::new()), - } + let addresses = self + .precompiles + .addresses() + .iter() + .copied() + .chain(core::iter::once(caller)) + .chain(core::iter::once(address)); + if let Err(e) = gasometer.access_addresses(addresses) { + return (e.into(), Vec::new()); } } From d5c078a0d053aec63557960c1f9c64b118de35cd Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Mon, 5 Jul 2021 10:18:59 -0400 Subject: [PATCH 10/38] Ensure precompile addresses are added to storage access in create calls too --- src/executor/stack/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 3427c5438..5385224d9 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -384,6 +384,11 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, try_or_fail!( self.state.metadata_mut().gasometer.access_address(address) ); + try_or_fail!( + self.state.metadata_mut().gasometer.access_addresses( + self.precompiles.addresses().iter().copied() + ) + ); event!(Create { caller, From a75b46f77d31c84af7133be29052134b8fc884b9 Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Mon, 5 Jul 2021 10:20:44 -0400 Subject: [PATCH 11/38] Fix benches compilation --- src/executor/stack/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 5385224d9..0a40f848f 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -106,7 +106,7 @@ pub trait Precompiles { } #[derive(Default)] -struct NoPrecompile { +pub struct NoPrecompile { addresses: Vec, } From 3674356d74e9902b5acd51fb8ff0f479acd8a3c9 Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Mon, 5 Jul 2021 11:04:43 -0400 Subject: [PATCH 12/38] Initialize accessed_addressed with caller too --- src/executor/stack/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 0a40f848f..fa7dbc97d 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -381,6 +381,9 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, let address = self.create_address(scheme); + try_or_fail!( + self.state.metadata_mut().gasometer.access_address(caller) + ); try_or_fail!( self.state.metadata_mut().gasometer.access_address(address) ); From 712e7c3606b773cdf98877ebc5bbdd38fc2d56bd Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Tue, 6 Jul 2021 15:28:40 +0700 Subject: [PATCH 13/38] Export Precompile at root --- src/executor/mod.rs | 2 +- src/executor/stack/mod.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/executor/mod.rs b/src/executor/mod.rs index 9a174ef79..4b4399b32 100644 --- a/src/executor/mod.rs +++ b/src/executor/mod.rs @@ -5,4 +5,4 @@ mod stack; -pub use self::stack::{StackExecutor, MemoryStackState, StackState, StackSubstateMetadata, StackExitKind, PrecompileOutput}; +pub use self::stack::{StackExecutor, MemoryStackState, StackState, StackSubstateMetadata, StackExitKind, PrecompileOutput, Precompile}; diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index fa7dbc97d..36d8606ab 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -89,7 +89,7 @@ pub struct PrecompileOutput { } /// Precompiles trait which allows for the execution of a precompile. -pub trait Precompiles { +pub trait Precompile { /// Runs the precompile at the given address with input and context. fn run<'config, S: StackState<'config>>( &self, @@ -110,7 +110,7 @@ pub struct NoPrecompile { addresses: Vec, } -impl Precompiles for NoPrecompile { +impl Precompile for NoPrecompile { fn run<'config, S: StackState<'config>>(&self, _address: H160, _input: &[u8], _gas_limit: Option,_context: &Context, _state: &mut S, _is_static: bool) -> Option> { None } @@ -121,7 +121,7 @@ impl Precompiles for NoPrecompile { } /// Stack-based executor. -pub struct StackExecutor<'config, S: StackState<'config>, P: Precompiles> { +pub struct StackExecutor<'config, S: StackState<'config>, P: Precompile> { config: &'config Config, precompiles: P, state: S, @@ -137,7 +137,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S, NoPrecompile> { } } -impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, P> { +impl<'config, S: StackState<'config>, P: Precompile> StackExecutor<'config, S, P> { /// Return a reference of the Config. pub fn config( &self @@ -660,7 +660,7 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, } } -impl<'config, S: StackState<'config>, P: Precompiles> Handler for StackExecutor<'config, S, P> { +impl<'config, S: StackState<'config>, P: Precompile> Handler for StackExecutor<'config, S, P> { type CreateInterrupt = Infallible; type CreateFeedback = Infallible; type CallInterrupt = Infallible; From 224a4719dfe988533e378829e0a38030c2042db3 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Tue, 6 Jul 2021 16:27:18 +0700 Subject: [PATCH 14/38] Enable EIP-2929 for Berlin --- runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index f81fee606..10c0e249a 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -360,7 +360,7 @@ impl Config { gas_storage_read_warm: 100, sstore_gas_metering: true, sstore_revert_under_stipend: true, - increase_state_access_gas: false, + increase_state_access_gas: true, err_on_call_with_more_gas: false, empty_considered_exists: false, create_increase_nonce: true, From 45de2a44dcd09d7d09b1d703aaeae41340c0c03e Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Tue, 6 Jul 2021 09:36:24 -0400 Subject: [PATCH 15/38] Update accessed_addresses and accessed_storage_keys on read/write; scope them by call --- gasometer/src/lib.rs | 105 ++++++++++++++++++++++++++++++-------- src/executor/stack/mod.rs | 9 +++- 2 files changed, 92 insertions(+), 22 deletions(-) diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 1b7d442c6..5e456acd8 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -81,18 +81,51 @@ impl<'config> Gasometer<'config> { config, accessed_addresses, accessed_storage_keys, + parent: None, }), } } + pub fn spawn(self, gas_limit: u64) -> Self { + let (accessed_addresses, accessed_storage_keys) = if self.config.increase_state_access_gas { + (Some(BTreeSet::new()), Some(BTreeSet::new())) + } else { + (None, None) + }; + let config = self.config; + + + Self { + gas_limit, + config, + inner: Ok(Inner { + memory_gas: 0, + used_gas: 0, + refunded_gas: 0, + config, + accessed_addresses, + accessed_storage_keys, + parent: self.inner.ok().map(Box::new), + }) + } + } + + pub fn get_accessed_addresses(&self) -> Option> { + self.inner.as_ref().ok().and_then(|inner| inner.accessed_addresses.as_ref()).map(|set| set.iter()) + } + + pub fn get_accessed_storages(&self) -> Option> { + self.inner.as_ref().ok().and_then(|inner| inner.accessed_storage_keys.as_ref()).map(|set| set.iter()) + } + #[inline] /// Returns the numerical gas cost value. pub fn gas_cost( - &self, + &mut self, cost: GasCost, gas: u64, ) -> Result { - match self.inner.as_ref() { + match self.inner.as_mut() { Ok(inner) => inner.gas_cost(cost, gas), Err(e) => Err(e.clone()) } @@ -266,10 +299,15 @@ impl<'config> Gasometer<'config> { Ok(()) } - pub fn access_storage(&mut self, address: H160, key: H256) -> Result<(), ExitError> { + pub fn access_storages(&mut self, iter: I) -> Result<(), ExitError> + where + I: Iterator + { let inner = self.inner_mut()?; if let Some(accessed_storage_keys) = &mut inner.accessed_storage_keys { - accessed_storage_keys.insert((address, key)); + for (address, key) in iter { + accessed_storage_keys.insert((address, key)); + } } Ok(()) } @@ -679,6 +717,7 @@ struct Inner<'config> { config: &'config Config, accessed_addresses: Option>, accessed_storage_keys: Option>, + parent: Option>>, } impl<'config> Inner<'config> { @@ -726,25 +765,25 @@ impl<'config> Inner<'config> { /// Returns the gas cost numerical value. fn gas_cost( - &self, + &mut self, cost: GasCost, gas: u64, ) -> Result { Ok(match cost { GasCost::Call { value, target, target_exists, .. } => - costs::call_cost(value, self.is_address_cold(target), true, true, !target_exists, self.config), + costs::call_cost(value, self.check_and_update_address_cold(target), true, true, !target_exists, self.config), GasCost::CallCode { value, target, target_exists, .. } => - costs::call_cost(value, self.is_address_cold(target), true, false, !target_exists, self.config), + costs::call_cost(value, self.check_and_update_address_cold(target), true, false, !target_exists, self.config), GasCost::DelegateCall { target, target_exists, .. } => - costs::call_cost(U256::zero(), self.is_address_cold(target), false, false, !target_exists, self.config), + costs::call_cost(U256::zero(), self.check_and_update_address_cold(target), false, false, !target_exists, self.config), GasCost::StaticCall { target, target_exists, .. } => - costs::call_cost(U256::zero(), self.is_address_cold(target), false, true, !target_exists, self.config), + costs::call_cost(U256::zero(), self.check_and_update_address_cold(target), false, true, !target_exists, self.config), GasCost::Suicide { value, target, target_exists, .. } => - costs::suicide_cost(value, self.is_address_cold(target), target_exists, self.config), + costs::suicide_cost(value, self.check_and_update_address_cold(target), target_exists, self.config), GasCost::SStore { .. } if self.config.estimate => self.config.gas_sstore_set, GasCost::SStore { address, key, original, current, new } => - costs::sstore_cost(original, current, new, gas, self.is_storage_cold(address, key), self.config)?, + costs::sstore_cost(original, current, new, gas, self.check_and_update_storage_cold(address, key), self.config)?, GasCost::Sha3 { len } => costs::sha3_cost(len)?, GasCost::Log { n, len } => costs::log_cost(n, len)?, @@ -753,7 +792,7 @@ impl<'config> Inner<'config> { GasCost::Create => consts::G_CREATE, GasCost::Create2 { len } => costs::create2_cost(len)?, GasCost::SLoad { address, key } => - costs::storage_access_cost(self.is_storage_cold(address, key), self.config.gas_sload, self.config), + costs::storage_access_cost(self.check_and_update_storage_cold(address, key), self.config.gas_sload, self.config), GasCost::Zero => consts::G_ZERO, GasCost::Base => consts::G_BASE, @@ -762,29 +801,53 @@ impl<'config> Inner<'config> { GasCost::Invalid => return Err(ExitError::OutOfGas), GasCost::ExtCodeSize { address } => - costs::storage_access_cost(self.is_address_cold(address), self.config.gas_ext_code, self.config), + costs::storage_access_cost(self.check_and_update_address_cold(address), self.config.gas_ext_code, self.config), GasCost::ExtCodeCopy { address, len } => - costs::extcodecopy_cost(len, self.is_address_cold(address), self.config)?, + costs::extcodecopy_cost(len, self.check_and_update_address_cold(address), self.config)?, GasCost::Balance { address } => - costs::storage_access_cost(self.is_address_cold(address), self.config.gas_balance, self.config), + costs::storage_access_cost(self.check_and_update_address_cold(address), self.config.gas_balance, self.config), GasCost::BlockHash => consts::G_BLOCKHASH, GasCost::ExtCodeHash { address } => - costs::storage_access_cost(self.is_address_cold(address), self.config.gas_ext_code_hash, self.config), + costs::storage_access_cost(self.check_and_update_address_cold(address), self.config.gas_ext_code_hash, self.config), }) } - fn is_address_cold(&self, address: H160) -> bool { + fn check_and_update_address_cold(&mut self, address: H160) -> bool { + let is_cold = self.is_address_cold(&address); + if is_cold { + self.accessed_addresses.as_mut().map(|set| set.insert(address)); + } + is_cold + } + + fn check_and_update_storage_cold(&mut self, address: H160, key: H256) -> bool { + let tuple = (address, key); + let is_cold = self.is_storage_cold(&tuple); + if is_cold { + self.accessed_storage_keys.as_mut().map(|set| set.insert(tuple)); + } + is_cold + } + + fn is_address_cold(&self, address: &H160) -> bool { if let Some(accessed_addresses) = &self.accessed_addresses { - !accessed_addresses.contains(&address) + if accessed_addresses.contains(address) { + false + } else { + self.parent.as_ref().map(|p| p.is_address_cold(address)).unwrap_or(true) + } } else { false } } - fn is_storage_cold(&self, address: H160, key: H256) -> bool { + fn is_storage_cold(&self, tuple: &(H160, H256)) -> bool { if let Some(accessed_storage_keys) = &self.accessed_storage_keys { - let tuple = (address, key); - !accessed_storage_keys.contains(&tuple) + if accessed_storage_keys.contains(tuple) { + false + } else { + self.parent.as_ref().map(|p| p.is_storage_cold(tuple)).unwrap_or(true) + } } else { false } diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 36d8606ab..4b607acaa 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -38,6 +38,12 @@ impl<'config> StackSubstateMetadata<'config> { pub fn swallow_commit(&mut self, other: Self) -> Result<(), ExitError> { self.gasometer.record_stipend(other.gasometer.gas())?; self.gasometer.record_refund(other.gasometer.refunded_gas())?; + if let Some(addresses) = other.gasometer.get_accessed_addresses() { + self.gasometer.access_addresses(addresses.copied())?; + } + if let Some(storages) = other.gasometer.get_accessed_storages() { + self.gasometer.access_storages(storages.copied())?; + } Ok(()) } @@ -54,7 +60,8 @@ impl<'config> StackSubstateMetadata<'config> { pub fn spit_child(&self, gas_limit: u64, is_static: bool) -> Self { Self { - gasometer: Gasometer::new(gas_limit, self.gasometer.config()), + // TODO: should be able to get rid of this clone + gasometer: self.gasometer.clone().spawn(gas_limit), is_static: is_static || self.is_static, depth: match self.depth { None => Some(0), From a2410a51b0918f8401814d25d2284c55f15cd76b Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Tue, 6 Jul 2021 10:19:41 -0400 Subject: [PATCH 16/38] Fix sload cost --- gasometer/src/costs.rs | 18 +++++++++++++++--- gasometer/src/lib.rs | 8 ++++---- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/gasometer/src/costs.rs b/gasometer/src/costs.rs index 8189cffd8..a523c146b 100644 --- a/gasometer/src/costs.rs +++ b/gasometer/src/costs.rs @@ -116,7 +116,7 @@ pub fn verylowcopy_cost(len: U256) -> Result { pub fn extcodecopy_cost(len: U256, is_cold: bool, config: &Config) -> Result { let wordd = len / U256::from(32); let wordr = len % U256::from(32); - let gas = U256::from(storage_access_cost(is_cold, config.gas_ext_code, config)).checked_add( + let gas = U256::from(address_access_cost(is_cold, config.gas_ext_code, config)).checked_add( U256::from(G_COPY).checked_mul( if wordr == U256::zero() { wordd @@ -168,6 +168,18 @@ pub fn sha3_cost(len: U256) -> Result { Ok(gas.as_u64()) } +pub fn sload_cost(is_cold: bool, config: &Config) -> u64 { + if config.increase_state_access_gas { + if is_cold { + config.gas_sload_cold + } else { + config.gas_storage_read_warm + } + } else { + config.gas_sload + } +} + pub fn sstore_cost(original: H256, current: H256, new: H256, gas: u64, is_cold: bool, config: &Config) -> Result { let (gas_sload, gas_sstore_reset) = if config.increase_state_access_gas { (config.gas_storage_read_warm, config.gas_sstore_reset - config.gas_sload_cold) @@ -241,12 +253,12 @@ pub fn call_cost( config: &Config, ) -> u64 { let transfers_value = value != U256::default(); - storage_access_cost(is_cold, config.gas_call, config) + + address_access_cost(is_cold, config.gas_call, config) + xfer_cost(is_call_or_callcode, transfers_value) + new_cost(is_call_or_staticcall, new_account, transfers_value, config) } -pub fn storage_access_cost(is_cold: bool, regular_value: u64, config: &Config) -> u64 { +pub fn address_access_cost(is_cold: bool, regular_value: u64, config: &Config) -> u64 { if config.increase_state_access_gas { if is_cold { config.gas_account_access_cold diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 5e456acd8..249c70177 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -792,7 +792,7 @@ impl<'config> Inner<'config> { GasCost::Create => consts::G_CREATE, GasCost::Create2 { len } => costs::create2_cost(len)?, GasCost::SLoad { address, key } => - costs::storage_access_cost(self.check_and_update_storage_cold(address, key), self.config.gas_sload, self.config), + costs::sload_cost(self.check_and_update_storage_cold(address, key), self.config), GasCost::Zero => consts::G_ZERO, GasCost::Base => consts::G_BASE, @@ -801,14 +801,14 @@ impl<'config> Inner<'config> { GasCost::Invalid => return Err(ExitError::OutOfGas), GasCost::ExtCodeSize { address } => - costs::storage_access_cost(self.check_and_update_address_cold(address), self.config.gas_ext_code, self.config), + costs::address_access_cost(self.check_and_update_address_cold(address), self.config.gas_ext_code, self.config), GasCost::ExtCodeCopy { address, len } => costs::extcodecopy_cost(len, self.check_and_update_address_cold(address), self.config)?, GasCost::Balance { address } => - costs::storage_access_cost(self.check_and_update_address_cold(address), self.config.gas_balance, self.config), + costs::address_access_cost(self.check_and_update_address_cold(address), self.config.gas_balance, self.config), GasCost::BlockHash => consts::G_BLOCKHASH, GasCost::ExtCodeHash { address } => - costs::storage_access_cost(self.check_and_update_address_cold(address), self.config.gas_ext_code_hash, self.config), + costs::address_access_cost(self.check_and_update_address_cold(address), self.config.gas_ext_code_hash, self.config), }) } From 40c102346ec492ef3cb2ac20f938c048d28792f0 Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Thu, 8 Jul 2021 15:45:29 +0000 Subject: [PATCH 17/38] Fix sstore refund --- gasometer/src/costs.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/gasometer/src/costs.rs b/gasometer/src/costs.rs index a523c146b..be5385721 100644 --- a/gasometer/src/costs.rs +++ b/gasometer/src/costs.rs @@ -37,10 +37,15 @@ pub fn sstore_refund(original: H256, current: H256, new: H256, config: &Config) } if original == new { + let (gas_sstore_reset, gas_sload) = if config.increase_state_access_gas { + (config.gas_sstore_reset - config.gas_sload_cold, config.gas_storage_read_warm) + } else { + (config.gas_sstore_reset, config.gas_sload) + }; if original == H256::default() { - refund += (config.gas_sstore_set - config.gas_sload) as i64; + refund += (config.gas_sstore_set - gas_sload) as i64; } else { - refund += (config.gas_sstore_reset - config.gas_sload) as i64; + refund += (gas_sstore_reset - gas_sload) as i64; } } From 3e081dc934d12d4405c082bcf2ff968ec8cc382d Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Fri, 9 Jul 2021 19:11:24 +0000 Subject: [PATCH 18/38] Fix random state test 649 (see https://github.com/ethereum/tests/commit/17f7e7a6c64bb878c1b6af9dc8371b46c133e46d) --- core/src/memory.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/src/memory.rs b/core/src/memory.rs index 01938696b..b874cb586 100644 --- a/core/src/memory.rs +++ b/core/src/memory.rs @@ -142,6 +142,11 @@ impl Memory { len: U256, data: &[u8] ) -> Result<(), ExitFatal> { + if len.is_zero() { + // a zero-length copy is defined to be a no-op + return Ok(()); + } + let memory_offset = if memory_offset > U256::from(usize::max_value()) { return Err(ExitFatal::NotSupported) } else { From 0b0782360c9230fd6b789c201ff2a1e26da9caf4 Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Fri, 9 Jul 2021 17:33:26 -0400 Subject: [PATCH 19/38] EIP-2930 support --- benches/loop.rs | 1 + gasometer/src/lib.rs | 44 ++++++++++++++++++++++++++++++--------- runtime/src/lib.rs | 10 +++++++++ src/executor/stack/mod.rs | 40 ++++++++++++++++++++++++++++++----- 4 files changed, 80 insertions(+), 15 deletions(-) diff --git a/benches/loop.rs b/benches/loop.rs index 621f18e97..316bbf3dc 100644 --- a/benches/loop.rs +++ b/benches/loop.rs @@ -52,6 +52,7 @@ fn run_loop_contract() { hex::decode("0f14a4060000000000000000000000000000000000000000000000000000000000b71b00").unwrap(), // hex::decode("0f14a4060000000000000000000000000000000000000000000000000000000000002ee0").unwrap(), u64::max_value(), + Vec::new(), ); } diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 249c70177..14de04c0d 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -318,15 +318,19 @@ impl<'config> Gasometer<'config> { cost: TransactionCost, ) -> Result<(), ExitError> { let gas_cost = match cost { - TransactionCost::Call { zero_data_len, non_zero_data_len } => { + TransactionCost::Call { zero_data_len, non_zero_data_len, access_list_address_len, access_list_storage_len } => { self.config.gas_transaction_call + zero_data_len as u64 * self.config.gas_transaction_zero_data + - non_zero_data_len as u64 * self.config.gas_transaction_non_zero_data + non_zero_data_len as u64 * self.config.gas_transaction_non_zero_data + + access_list_address_len as u64 * self.config.gas_access_list_address + + access_list_storage_len as u64 * self.config.gas_access_list_storage_key }, - TransactionCost::Create { zero_data_len, non_zero_data_len } => { + TransactionCost::Create { zero_data_len, non_zero_data_len, access_list_address_len, access_list_storage_len } => { self.config.gas_transaction_create + zero_data_len as u64 * self.config.gas_transaction_zero_data + - non_zero_data_len as u64 * self.config.gas_transaction_non_zero_data + non_zero_data_len as u64 * self.config.gas_transaction_non_zero_data + + access_list_address_len as u64 * self.config.gas_access_list_address + + access_list_storage_len as u64 * self.config.gas_access_list_storage_key }, }; @@ -357,22 +361,34 @@ impl<'config> Gasometer<'config> { /// Calculate the call transaction cost. pub fn call_transaction_cost( - data: &[u8] + data: &[u8], + access_list: &[(H160, Vec)], ) -> TransactionCost { let zero_data_len = data.iter().filter(|v| **v == 0).count(); let non_zero_data_len = data.len() - zero_data_len; + let (access_list_address_len, access_list_storage_len) = count_access_list(access_list); - TransactionCost::Call { zero_data_len, non_zero_data_len } + TransactionCost::Call { zero_data_len, non_zero_data_len, access_list_address_len, access_list_storage_len } } /// Calculate the create transaction cost. pub fn create_transaction_cost( - data: &[u8] + data: &[u8], + access_list: &[(H160, Vec)], ) -> TransactionCost { let zero_data_len = data.iter().filter(|v| **v == 0).count(); let non_zero_data_len = data.len() - zero_data_len; + let (access_list_address_len, access_list_storage_len) = count_access_list(access_list); - TransactionCost::Create { zero_data_len, non_zero_data_len } + TransactionCost::Create { zero_data_len, non_zero_data_len, access_list_address_len, access_list_storage_len } +} + +/// Counts the number of addresses and storage keys in the access list +fn count_access_list(access_list: &[(H160, Vec)]) -> (usize, usize) { + let access_list_address_len = access_list.len(); + let access_list_storage_len = access_list.iter().map(|(_, keys)| keys.len()).sum(); + + (access_list_address_len, access_list_storage_len) } #[inline] @@ -1027,14 +1043,22 @@ pub enum TransactionCost { /// Length of zeros in transaction data. zero_data_len: usize, /// Length of non-zeros in transaction data. - non_zero_data_len: usize + non_zero_data_len: usize, + /// Number of addresses in transaction access list (see EIP-2930) + access_list_address_len: usize, + /// Total number of storage keys in transaction access list (see EIP-2930) + access_list_storage_len: usize, }, /// Create transaction cost. Create { /// Length of zeros in transaction data. zero_data_len: usize, /// Length of non-zeros in transaction data. - non_zero_data_len: usize + non_zero_data_len: usize, + /// Number of addresses in transaction access list (see EIP-2930) + access_list_address_len: usize, + /// Total number of storage keys in transaction access list (see EIP-2930) + access_list_storage_len: usize, }, } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 10c0e249a..69688051d 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -196,6 +196,10 @@ pub struct Config { pub gas_transaction_zero_data: u64, /// Gas paid for non-zero data in a transaction. pub gas_transaction_non_zero_data: u64, + /// Gas paid per address in transaction access list (see EIP-2930). + pub gas_access_list_address: u64, + /// Gas paid per storage key in transaction access list (see EIP-2930). + pub gas_access_list_storage_key: u64, /// Gas paid for accessing cold account. pub gas_account_access_cold: u64, /// Gas paid for accessing ready storage. @@ -266,6 +270,8 @@ impl Config { gas_transaction_call: 21000, gas_transaction_zero_data: 4, gas_transaction_non_zero_data: 68, + gas_access_list_address: 0, + gas_access_list_storage_key: 0, gas_account_access_cold: 0, gas_storage_read_warm: 0, sstore_gas_metering: false, @@ -311,6 +317,8 @@ impl Config { gas_transaction_call: 21000, gas_transaction_zero_data: 4, gas_transaction_non_zero_data: 16, + gas_access_list_address: 0, + gas_access_list_storage_key: 0, gas_account_access_cold: 0, gas_storage_read_warm: 0, sstore_gas_metering: true, @@ -356,6 +364,8 @@ impl Config { gas_transaction_call: 21000, gas_transaction_zero_data: 4, gas_transaction_non_zero_data: 16, + gas_access_list_address: 2400, + gas_access_list_storage_key: 1900, gas_account_access_cold: 2600, gas_storage_read_warm: 100, sstore_gas_metering: true, diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 4b607acaa..9c4b6f73d 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -218,13 +218,19 @@ impl<'config, S: StackState<'config>, P: Precompile> StackExecutor<'config, S, P value: U256, init_code: Vec, gas_limit: u64, + access_list: Vec<(H160, Vec)>, // See EIP-2930 ) -> ExitReason { - let transaction_cost = gasometer::create_transaction_cost(&init_code); - match self.state.metadata_mut().gasometer.record_transaction(transaction_cost) { + let transaction_cost = gasometer::create_transaction_cost(&init_code, &access_list); + let gasometer = &mut self.state.metadata_mut().gasometer; + match gasometer.record_transaction(transaction_cost) { Ok(()) => (), Err(e) => return e.into(), } + if let Err(e) = Self::initialize_with_access_list(gasometer, access_list) { + return e.into(); + } + match self.create_inner( caller, CreateScheme::Legacy { caller }, @@ -246,14 +252,20 @@ impl<'config, S: StackState<'config>, P: Precompile> StackExecutor<'config, S, P init_code: Vec, salt: H256, gas_limit: u64, + access_list: Vec<(H160, Vec)>, // See EIP-2930 ) -> ExitReason { - let transaction_cost = gasometer::create_transaction_cost(&init_code); - match self.state.metadata_mut().gasometer.record_transaction(transaction_cost) { + let transaction_cost = gasometer::create_transaction_cost(&init_code, &access_list); + let gasometer = &mut self.state.metadata_mut().gasometer; + match gasometer.record_transaction(transaction_cost) { Ok(()) => (), Err(e) => return e.into(), } let code_hash = H256::from_slice(Keccak256::digest(&init_code).as_slice()); + if let Err(e) = Self::initialize_with_access_list(gasometer, access_list) { + return e.into(); + } + match self.create_inner( caller, CreateScheme::Create2 { caller, code_hash, salt }, @@ -275,8 +287,9 @@ impl<'config, S: StackState<'config>, P: Precompile> StackExecutor<'config, S, P value: U256, data: Vec, gas_limit: u64, + access_list: Vec<(H160, Vec)>, // See EIP-2930 ) -> (ExitReason, Vec) { - let transaction_cost = gasometer::call_transaction_cost(&data); + let transaction_cost = gasometer::call_transaction_cost(&data, &access_list); let gasometer = &mut self.state.metadata_mut().gasometer; match gasometer @@ -297,6 +310,10 @@ impl<'config, S: StackState<'config>, P: Precompile> StackExecutor<'config, S, P if let Err(e) = gasometer.access_addresses(addresses) { return (e.into(), Vec::new()); } + + if let Err(e) = Self::initialize_with_access_list(gasometer, access_list) { + return (e.into(), Vec::new()); + } } self.state.inc_nonce(caller); @@ -364,6 +381,19 @@ impl<'config, S: StackState<'config>, P: Precompile> StackExecutor<'config, S, P } } + fn initialize_with_access_list( + gasometer: &mut Gasometer, + access_list: Vec<(H160, Vec)> + ) -> Result<(), ExitError> { + let addresses = access_list.iter().map(|a| a.0); + gasometer.access_addresses(addresses)?; + + let storage_keys = access_list + .into_iter() + .flat_map(|(address, keys)| keys.into_iter().map(move |key| (address, key))); + gasometer.access_storages(storage_keys) + } + fn create_inner( &mut self, caller: H160, From 7a74753992d42a52abe74a1d7f49b7a872285533 Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Tue, 13 Jul 2021 15:21:27 +0000 Subject: [PATCH 20/38] Fix bug in EIP-1706 implementation. Note the spec says _less than or equal to_ --- gasometer/src/costs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gasometer/src/costs.rs b/gasometer/src/costs.rs index be5385721..194ba5f13 100644 --- a/gasometer/src/costs.rs +++ b/gasometer/src/costs.rs @@ -193,7 +193,7 @@ pub fn sstore_cost(original: H256, current: H256, new: H256, gas: u64, is_cold: }; let gas_cost = if config.sstore_gas_metering { if config.sstore_revert_under_stipend { - if gas < config.call_stipend { + if gas <= config.call_stipend { return Err(ExitError::OutOfGas) } } From bf2fb7749114501fd25c6e716f7f488bcc7a0705 Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Wed, 14 Jul 2021 13:48:33 +0000 Subject: [PATCH 21/38] Import alloc types --- gasometer/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 14de04c0d..5c2af3e2f 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -28,7 +28,9 @@ mod costs; mod memory; mod utils; +use alloc::boxed::Box; use alloc::collections::BTreeSet; +use alloc::vec::Vec; use core::cmp::max; use primitive_types::{H160, H256, U256}; use evm_core::{Opcode, ExitError, Stack}; From 09d4fe09dcb5fcabed8c1076699c8a2e70f14c23 Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Wed, 14 Jul 2021 14:39:49 +0000 Subject: [PATCH 22/38] Precompiles trait refactor --- src/executor/mod.rs | 2 +- src/executor/stack/mod.rs | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/executor/mod.rs b/src/executor/mod.rs index 4b4399b32..fffa5e56c 100644 --- a/src/executor/mod.rs +++ b/src/executor/mod.rs @@ -5,4 +5,4 @@ mod stack; -pub use self::stack::{StackExecutor, MemoryStackState, StackState, StackSubstateMetadata, StackExitKind, PrecompileOutput, Precompile}; +pub use self::stack::{StackExecutor, MemoryStackState, StackState, StackSubstateMetadata, StackExitKind, PrecompileOutput, Precompiles}; diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 9c4b6f73d..538ee5102 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -96,9 +96,9 @@ pub struct PrecompileOutput { } /// Precompiles trait which allows for the execution of a precompile. -pub trait Precompile { +pub trait Precompiles { /// Runs the precompile at the given address with input and context. - fn run<'config, S: StackState<'config>>( + fn run( &self, address: H160, input: &[u8], @@ -117,8 +117,8 @@ pub struct NoPrecompile { addresses: Vec, } -impl Precompile for NoPrecompile { - fn run<'config, S: StackState<'config>>(&self, _address: H160, _input: &[u8], _gas_limit: Option,_context: &Context, _state: &mut S, _is_static: bool) -> Option> { +impl Precompiles for NoPrecompile { + fn run(&self, _address: H160, _input: &[u8], _gas_limit: Option,_context: &Context, _state: &mut S, _is_static: bool) -> Option> { None } @@ -128,7 +128,7 @@ impl Precompile for NoPrecompile { } /// Stack-based executor. -pub struct StackExecutor<'config, S: StackState<'config>, P: Precompile> { +pub struct StackExecutor<'config, S: StackState<'config>, P: Precompiles> { config: &'config Config, precompiles: P, state: S, @@ -144,7 +144,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S, NoPrecompile> { } } -impl<'config, S: StackState<'config>, P: Precompile> StackExecutor<'config, S, P> { +impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, P> { /// Return a reference of the Config. pub fn config( &self @@ -697,7 +697,7 @@ impl<'config, S: StackState<'config>, P: Precompile> StackExecutor<'config, S, P } } -impl<'config, S: StackState<'config>, P: Precompile> Handler for StackExecutor<'config, S, P> { +impl<'config, S: StackState<'config>, P: Precompiles> Handler for StackExecutor<'config, S, P> { type CreateInterrupt = Infallible; type CreateFeedback = Infallible; type CallInterrupt = Infallible; From f230e7d312f070a687ce3736d33057f1f2dd1505 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Wed, 14 Jul 2021 22:51:55 +0700 Subject: [PATCH 23/38] Update `transact_call` documentation --- src/executor/stack/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 538ee5102..93c923813 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -279,7 +279,12 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, } } - /// Execute a `CALL` transaction. + /// Execute a `CALL` transaction with a given caller, address, value and + /// gas limit and data. + /// + /// Takes in an additional `access_list` parameter for EIP-2930 which was + /// introduced in the Ethereum Berlin hard fork. If you do not wish to use + /// this functionality, just pass in an empty vector. pub fn transact_call( &mut self, caller: H160, @@ -287,7 +292,7 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, value: U256, data: Vec, gas_limit: u64, - access_list: Vec<(H160, Vec)>, // See EIP-2930 + access_list: Vec<(H160, Vec)>, ) -> (ExitReason, Vec) { let transaction_cost = gasometer::call_transaction_cost(&data, &access_list); From 384ff221bf833e01794b8debd9d36cdcdcfffe23 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Tue, 27 Jul 2021 14:01:19 +0700 Subject: [PATCH 24/38] cargo fmt --- core/src/memory.rs | 8 +- gasometer/src/costs.rs | 56 ++++---- gasometer/src/lib.rs | 267 ++++++++++++++++++++++++++------------ runtime/src/lib.rs | 2 +- src/executor/mod.rs | 5 +- src/executor/stack/mod.rs | 80 +++++++----- 6 files changed, 276 insertions(+), 142 deletions(-) diff --git a/core/src/memory.rs b/core/src/memory.rs index 0da23a25c..5fe1e1cf7 100644 --- a/core/src/memory.rs +++ b/core/src/memory.rs @@ -144,10 +144,10 @@ impl Memory { len: U256, data: &[u8], ) -> Result<(), ExitFatal> { - if len.is_zero() { - // a zero-length copy is defined to be a no-op - return Ok(()); - } + if len.is_zero() { + // a zero-length copy is defined to be a no-op + return Ok(()); + } let memory_offset = if memory_offset > U256::from(usize::MAX) { return Err(ExitFatal::NotSupported); diff --git a/gasometer/src/costs.rs b/gasometer/src/costs.rs index b179ea19a..2aca073f3 100644 --- a/gasometer/src/costs.rs +++ b/gasometer/src/costs.rs @@ -38,7 +38,10 @@ pub fn sstore_refund(original: H256, current: H256, new: H256, config: &Config) if original == new { let (gas_sstore_reset, gas_sload) = if config.increase_state_access_gas { - (config.gas_sstore_reset - config.gas_sload_cold, config.gas_storage_read_warm) + ( + config.gas_sstore_reset - config.gas_sload_cold, + config.gas_storage_read_warm, + ) } else { (config.gas_sstore_reset, config.gas_sload) }; @@ -128,15 +131,17 @@ pub fn verylowcopy_cost(len: U256) -> Result { pub fn extcodecopy_cost(len: U256, is_cold: bool, config: &Config) -> Result { let wordd = len / U256::from(32); let wordr = len % U256::from(32); - let gas = U256::from(address_access_cost(is_cold, config.gas_ext_code, config)).checked_add( - U256::from(G_COPY).checked_mul( - if wordr == U256::zero() { - wordd - } else { - wordd + U256::one() - } - ).ok_or(ExitError::OutOfGas)? - ).ok_or(ExitError::OutOfGas)?; + let gas = U256::from(address_access_cost(is_cold, config.gas_ext_code, config)) + .checked_add( + U256::from(G_COPY) + .checked_mul(if wordr == U256::zero() { + wordd + } else { + wordd + U256::one() + }) + .ok_or(ExitError::OutOfGas)?, + ) + .ok_or(ExitError::OutOfGas)?; if gas > U256::from(u64::MAX) { return Err(ExitError::OutOfGas); @@ -198,16 +203,26 @@ pub fn sload_cost(is_cold: bool, config: &Config) -> u64 { } } -pub fn sstore_cost(original: H256, current: H256, new: H256, gas: u64, is_cold: bool, config: &Config) -> Result { - let (gas_sload, gas_sstore_reset) = if config.increase_state_access_gas { - (config.gas_storage_read_warm, config.gas_sstore_reset - config.gas_sload_cold) +pub fn sstore_cost( + original: H256, + current: H256, + new: H256, + gas: u64, + is_cold: bool, + config: &Config, +) -> Result { + let (gas_sload, gas_sstore_reset) = if config.increase_state_access_gas { + ( + config.gas_storage_read_warm, + config.gas_sstore_reset - config.gas_sload_cold, + ) } else { (config.gas_sload, config.gas_sstore_reset) }; let gas_cost = if config.sstore_gas_metering { if config.sstore_revert_under_stipend { if gas <= config.call_stipend { - return Err(ExitError::OutOfGas) + return Err(ExitError::OutOfGas); } } @@ -237,7 +252,7 @@ pub fn sstore_cost(original: H256, current: H256, new: H256, gas: u64, is_cold: gas_cost + config.gas_sload_cold } else { gas_cost - } + }, ) } @@ -271,9 +286,9 @@ pub fn call_cost( config: &Config, ) -> u64 { let transfers_value = value != U256::default(); - address_access_cost(is_cold, config.gas_call, config) + - xfer_cost(is_call_or_callcode, transfers_value) + - new_cost(is_call_or_staticcall, new_account, transfers_value, config) + address_access_cost(is_cold, config.gas_call, config) + + xfer_cost(is_call_or_callcode, transfers_value) + + new_cost(is_call_or_staticcall, new_account, transfers_value, config) } pub fn address_access_cost(is_cold: bool, regular_value: u64, config: &Config) -> u64 { @@ -288,10 +303,7 @@ pub fn address_access_cost(is_cold: bool, regular_value: u64, config: &Config) - } } -fn xfer_cost( - is_call_or_callcode: bool, - transfers_value: bool -) -> u64 { +fn xfer_cost(is_call_or_callcode: bool, transfers_value: bool) -> u64 { if is_call_or_callcode && transfers_value { G_CALLVALUE } else { diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index a11afb49c..bd9532472 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -95,7 +95,6 @@ impl<'config> Gasometer<'config> { }; let config = self.config; - Self { gas_limit, config, @@ -107,25 +106,31 @@ impl<'config> Gasometer<'config> { accessed_addresses, accessed_storage_keys, parent: self.inner.ok().map(Box::new), - }) + }), } } pub fn get_accessed_addresses(&self) -> Option> { - self.inner.as_ref().ok().and_then(|inner| inner.accessed_addresses.as_ref()).map(|set| set.iter()) + self.inner + .as_ref() + .ok() + .and_then(|inner| inner.accessed_addresses.as_ref()) + .map(|set| set.iter()) } - pub fn get_accessed_storages(&self) -> Option> { - self.inner.as_ref().ok().and_then(|inner| inner.accessed_storage_keys.as_ref()).map(|set| set.iter()) + pub fn get_accessed_storages( + &self, + ) -> Option> { + self.inner + .as_ref() + .ok() + .and_then(|inner| inner.accessed_storage_keys.as_ref()) + .map(|set| set.iter()) } #[inline] /// Returns the numerical gas cost value. - pub fn gas_cost( - &mut self, - cost: GasCost, - gas: u64, - ) -> Result { + pub fn gas_cost(&mut self, cost: GasCost, gas: u64) -> Result { match self.inner.as_mut() { Ok(inner) => inner.gas_cost(cost, gas), Err(e) => Err(e.clone()), @@ -275,7 +280,7 @@ impl<'config> Gasometer<'config> { pub fn access_addresses(&mut self, addresses: I) -> Result<(), ExitError> where - I: Iterator + I: Iterator, { let inner = self.inner_mut()?; if let Some(accessed_addresses) = &mut inner.accessed_addresses { @@ -288,10 +293,10 @@ impl<'config> Gasometer<'config> { pub fn access_storages(&mut self, iter: I) -> Result<(), ExitError> where - I: Iterator + I: Iterator, { let inner = self.inner_mut()?; - if let Some(accessed_storage_keys) = &mut inner.accessed_storage_keys { + if let Some(accessed_storage_keys) = &mut inner.accessed_storage_keys { for (address, key) in iter { accessed_storage_keys.insert((address, key)); } @@ -302,20 +307,30 @@ impl<'config> Gasometer<'config> { /// Record transaction cost. pub fn record_transaction(&mut self, cost: TransactionCost) -> Result<(), ExitError> { let gas_cost = match cost { - TransactionCost::Call { zero_data_len, non_zero_data_len, access_list_address_len, access_list_storage_len } => { - self.config.gas_transaction_call + - zero_data_len as u64 * self.config.gas_transaction_zero_data + - non_zero_data_len as u64 * self.config.gas_transaction_non_zero_data + - access_list_address_len as u64 * self.config.gas_access_list_address + - access_list_storage_len as u64 * self.config.gas_access_list_storage_key - }, - TransactionCost::Create { zero_data_len, non_zero_data_len, access_list_address_len, access_list_storage_len } => { - self.config.gas_transaction_create + - zero_data_len as u64 * self.config.gas_transaction_zero_data + - non_zero_data_len as u64 * self.config.gas_transaction_non_zero_data + - access_list_address_len as u64 * self.config.gas_access_list_address + - access_list_storage_len as u64 * self.config.gas_access_list_storage_key - }, + TransactionCost::Call { + zero_data_len, + non_zero_data_len, + access_list_address_len, + access_list_storage_len, + } => { + self.config.gas_transaction_call + + zero_data_len as u64 * self.config.gas_transaction_zero_data + + non_zero_data_len as u64 * self.config.gas_transaction_non_zero_data + + access_list_address_len as u64 * self.config.gas_access_list_address + + access_list_storage_len as u64 * self.config.gas_access_list_storage_key + } + TransactionCost::Create { + zero_data_len, + non_zero_data_len, + access_list_address_len, + access_list_storage_len, + } => { + self.config.gas_transaction_create + + zero_data_len as u64 * self.config.gas_transaction_zero_data + + non_zero_data_len as u64 * self.config.gas_transaction_non_zero_data + + access_list_address_len as u64 * self.config.gas_access_list_address + + access_list_storage_len as u64 * self.config.gas_access_list_storage_key + } }; event!(RecordTransaction { @@ -344,27 +359,31 @@ impl<'config> Gasometer<'config> { } /// Calculate the call transaction cost. -pub fn call_transaction_cost( - data: &[u8], - access_list: &[(H160, Vec)], -) -> TransactionCost { +pub fn call_transaction_cost(data: &[u8], access_list: &[(H160, Vec)]) -> TransactionCost { let zero_data_len = data.iter().filter(|v| **v == 0).count(); let non_zero_data_len = data.len() - zero_data_len; let (access_list_address_len, access_list_storage_len) = count_access_list(access_list); - TransactionCost::Call { zero_data_len, non_zero_data_len, access_list_address_len, access_list_storage_len } + TransactionCost::Call { + zero_data_len, + non_zero_data_len, + access_list_address_len, + access_list_storage_len, + } } /// Calculate the create transaction cost. -pub fn create_transaction_cost( - data: &[u8], - access_list: &[(H160, Vec)], -) -> TransactionCost { +pub fn create_transaction_cost(data: &[u8], access_list: &[(H160, Vec)]) -> TransactionCost { let zero_data_len = data.iter().filter(|v| **v == 0).count(); let non_zero_data_len = data.len() - zero_data_len; let (access_list_address_len, access_list_storage_len) = count_access_list(access_list); - TransactionCost::Create { zero_data_len, non_zero_data_len, access_list_address_len, access_list_storage_len } + TransactionCost::Create { + zero_data_len, + non_zero_data_len, + access_list_address_len, + access_list_storage_len, + } } /// Counts the number of addresses and storage keys in the access list @@ -634,8 +653,9 @@ pub fn dynamic_opcode_cost( } } Opcode::CALL - if !is_static || - (is_static && U256::from_big_endian(&stack.peek(2)?[..]) == U256::zero()) => { + if !is_static + || (is_static && U256::from_big_endian(&stack.peek(2)?[..]) == U256::zero()) => + { let target = stack.peek(1)?.into(); GasCost::Call { value: U256::from_big_endian(&stack.peek(2)?[..]), @@ -761,26 +781,85 @@ impl<'config> Inner<'config> { } /// Returns the gas cost numerical value. - fn gas_cost( - &mut self, - cost: GasCost, - gas: u64, - ) -> Result { + fn gas_cost(&mut self, cost: GasCost, gas: u64) -> Result { Ok(match cost { - GasCost::Call { value, target, target_exists, .. } => - costs::call_cost(value, self.check_and_update_address_cold(target), true, true, !target_exists, self.config), - GasCost::CallCode { value, target, target_exists, .. } => - costs::call_cost(value, self.check_and_update_address_cold(target), true, false, !target_exists, self.config), - GasCost::DelegateCall { target, target_exists, .. } => - costs::call_cost(U256::zero(), self.check_and_update_address_cold(target), false, false, !target_exists, self.config), - GasCost::StaticCall { target, target_exists, .. } => - costs::call_cost(U256::zero(), self.check_and_update_address_cold(target), false, true, !target_exists, self.config), - - GasCost::Suicide { value, target, target_exists, .. } => - costs::suicide_cost(value, self.check_and_update_address_cold(target), target_exists, self.config), + GasCost::Call { + value, + target, + target_exists, + .. + } => costs::call_cost( + value, + self.check_and_update_address_cold(target), + true, + true, + !target_exists, + self.config, + ), + GasCost::CallCode { + value, + target, + target_exists, + .. + } => costs::call_cost( + value, + self.check_and_update_address_cold(target), + true, + false, + !target_exists, + self.config, + ), + GasCost::DelegateCall { + target, + target_exists, + .. + } => costs::call_cost( + U256::zero(), + self.check_and_update_address_cold(target), + false, + false, + !target_exists, + self.config, + ), + GasCost::StaticCall { + target, + target_exists, + .. + } => costs::call_cost( + U256::zero(), + self.check_and_update_address_cold(target), + false, + true, + !target_exists, + self.config, + ), + + GasCost::Suicide { + value, + target, + target_exists, + .. + } => costs::suicide_cost( + value, + self.check_and_update_address_cold(target), + target_exists, + self.config, + ), GasCost::SStore { .. } if self.config.estimate => self.config.gas_sstore_set, - GasCost::SStore { address, key, original, current, new } => - costs::sstore_cost(original, current, new, gas, self.check_and_update_storage_cold(address, key), self.config)?, + GasCost::SStore { + address, + key, + original, + current, + new, + } => costs::sstore_cost( + original, + current, + new, + gas, + self.check_and_update_storage_cold(address, key), + self.config, + )?, GasCost::Sha3 { len } => costs::sha3_cost(len)?, GasCost::Log { n, len } => costs::log_cost(n, len)?, @@ -788,8 +867,10 @@ impl<'config> Inner<'config> { GasCost::Exp { power } => costs::exp_cost(power, self.config)?, GasCost::Create => consts::G_CREATE, GasCost::Create2 { len } => costs::create2_cost(len)?, - GasCost::SLoad { address, key } => - costs::sload_cost(self.check_and_update_storage_cold(address, key), self.config), + GasCost::SLoad { address, key } => costs::sload_cost( + self.check_and_update_storage_cold(address, key), + self.config, + ), GasCost::Zero => consts::G_ZERO, GasCost::Base => consts::G_BASE, @@ -797,22 +878,36 @@ impl<'config> Inner<'config> { GasCost::Low => consts::G_LOW, GasCost::Invalid => return Err(ExitError::OutOfGas), - GasCost::ExtCodeSize { address } => - costs::address_access_cost(self.check_and_update_address_cold(address), self.config.gas_ext_code, self.config), - GasCost::ExtCodeCopy { address, len } => - costs::extcodecopy_cost(len, self.check_and_update_address_cold(address), self.config)?, - GasCost::Balance { address } => - costs::address_access_cost(self.check_and_update_address_cold(address), self.config.gas_balance, self.config), + GasCost::ExtCodeSize { address } => costs::address_access_cost( + self.check_and_update_address_cold(address), + self.config.gas_ext_code, + self.config, + ), + GasCost::ExtCodeCopy { address, len } => costs::extcodecopy_cost( + len, + self.check_and_update_address_cold(address), + self.config, + )?, + GasCost::Balance { address } => costs::address_access_cost( + self.check_and_update_address_cold(address), + self.config.gas_balance, + self.config, + ), GasCost::BlockHash => consts::G_BLOCKHASH, - GasCost::ExtCodeHash { address } => - costs::address_access_cost(self.check_and_update_address_cold(address), self.config.gas_ext_code_hash, self.config), + GasCost::ExtCodeHash { address } => costs::address_access_cost( + self.check_and_update_address_cold(address), + self.config.gas_ext_code_hash, + self.config, + ), }) } fn check_and_update_address_cold(&mut self, address: H160) -> bool { let is_cold = self.is_address_cold(&address); if is_cold { - self.accessed_addresses.as_mut().map(|set| set.insert(address)); + self.accessed_addresses + .as_mut() + .map(|set| set.insert(address)); } is_cold } @@ -821,17 +916,22 @@ impl<'config> Inner<'config> { let tuple = (address, key); let is_cold = self.is_storage_cold(&tuple); if is_cold { - self.accessed_storage_keys.as_mut().map(|set| set.insert(tuple)); + self.accessed_storage_keys + .as_mut() + .map(|set| set.insert(tuple)); } is_cold } fn is_address_cold(&self, address: &H160) -> bool { - if let Some(accessed_addresses) = &self.accessed_addresses { + if let Some(accessed_addresses) = &self.accessed_addresses { if accessed_addresses.contains(address) { false } else { - self.parent.as_ref().map(|p| p.is_address_cold(address)).unwrap_or(true) + self.parent + .as_ref() + .map(|p| p.is_address_cold(address)) + .unwrap_or(true) } } else { false @@ -839,28 +939,33 @@ impl<'config> Inner<'config> { } fn is_storage_cold(&self, tuple: &(H160, H256)) -> bool { - if let Some(accessed_storage_keys) = &self.accessed_storage_keys { + if let Some(accessed_storage_keys) = &self.accessed_storage_keys { if accessed_storage_keys.contains(tuple) { false } else { - self.parent.as_ref().map(|p| p.is_storage_cold(tuple)).unwrap_or(true) + self.parent + .as_ref() + .map(|p| p.is_storage_cold(tuple)) + .unwrap_or(true) } } else { false } } - fn gas_refund( - &self, - cost: GasCost - ) -> i64 { + fn gas_refund(&self, cost: GasCost) -> i64 { match cost { _ if self.config.estimate => 0, - GasCost::SStore { original, current, new, .. } => - costs::sstore_refund(original, current, new, self.config), - GasCost::Suicide { already_removed, .. } => - costs::suicide_refund(already_removed), + GasCost::SStore { + original, + current, + new, + .. + } => costs::sstore_refund(original, current, new, self.config), + GasCost::Suicide { + already_removed, .. + } => costs::suicide_refund(already_removed), _ => 0, } } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index c569ad8d8..753711888 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -178,7 +178,7 @@ pub struct Config { /// Gas paid for SLOAD opcode. pub gas_sload: u64, /// Gas paid for cold SLOAD opcode. - pub gas_sload_cold: u64, + pub gas_sload_cold: u64, /// Gas paid for SUICIDE opcode. pub gas_suicide: u64, /// Gas paid for SUICIDE opcode when it hits a new account. diff --git a/src/executor/mod.rs b/src/executor/mod.rs index fffa5e56c..792274c15 100644 --- a/src/executor/mod.rs +++ b/src/executor/mod.rs @@ -5,4 +5,7 @@ mod stack; -pub use self::stack::{StackExecutor, MemoryStackState, StackState, StackSubstateMetadata, StackExitKind, PrecompileOutput, Precompiles}; +pub use self::stack::{ + MemoryStackState, PrecompileOutput, Precompiles, StackExecutor, StackExitKind, StackState, + StackSubstateMetadata, +}; diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index fd6063dfb..0b3ea6b0e 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -36,7 +36,8 @@ impl<'config> StackSubstateMetadata<'config> { pub fn swallow_commit(&mut self, other: Self) -> Result<(), ExitError> { self.gasometer.record_stipend(other.gasometer.gas())?; - self.gasometer.record_refund(other.gasometer.refunded_gas())?; + self.gasometer + .record_refund(other.gasometer.refunded_gas())?; if let Some(addresses) = other.gasometer.get_accessed_addresses() { self.gasometer.access_addresses(addresses.copied())?; } @@ -101,7 +102,7 @@ pub trait Precompiles { &self, address: H160, input: &[u8], - gas_limit: Option, + gas_limit: Option, context: &Context, state: &mut S, is_static: bool, @@ -117,7 +118,15 @@ pub struct NoPrecompile { } impl Precompiles for NoPrecompile { - fn run(&self, _address: H160, _input: &[u8], _gas_limit: Option,_context: &Context, _state: &mut S, _is_static: bool) -> Option> { + fn run( + &self, + _address: H160, + _input: &[u8], + _gas_limit: Option, + _context: &Context, + _state: &mut S, + _is_static: bool, + ) -> Option> { None } @@ -135,10 +144,7 @@ pub struct StackExecutor<'config, S: StackState<'config>, P: Precompiles> { impl<'config, S: StackState<'config>> StackExecutor<'config, S, NoPrecompile> { /// Create a new stack-based executor. - pub fn new( - state: S, - config: &'config Config, - ) -> Self { + pub fn new(state: S, config: &'config Config) -> Self { Self::new_with_precompile(state, config, NoPrecompile::default()) } } @@ -150,11 +156,7 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, } /// Create a new stack-based executor with given precompiles. - pub fn new_with_precompile( - state: S, - config: &'config Config, - precompile: P, - ) -> Self { + pub fn new_with_precompile(state: S, config: &'config Config, precompile: P) -> Self { Self { config, precompiles: precompile, @@ -291,13 +293,12 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, let transaction_cost = gasometer::call_transaction_cost(&data, &access_list); let gasometer = &mut self.state.metadata_mut().gasometer; - match gasometer - .record_transaction(transaction_cost) { + match gasometer.record_transaction(transaction_cost) { Ok(()) => (), Err(e) => return (e.into(), Vec::new()), } - // Initialize initial addresses for EIP-2929 + // Initialize initial addresses for EIP-2929 if self.config.increase_state_access_gas { let addresses = self .precompiles @@ -390,7 +391,7 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, fn initialize_with_access_list( gasometer: &mut Gasometer, - access_list: Vec<(H160, Vec)> + access_list: Vec<(H160, Vec)>, ) -> Result<(), ExitError> { let addresses = access_list.iter().map(|a| a.0); gasometer.access_addresses(addresses)?; @@ -425,17 +426,13 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, let address = self.create_address(scheme); - try_or_fail!( - self.state.metadata_mut().gasometer.access_address(caller) - ); - try_or_fail!( - self.state.metadata_mut().gasometer.access_address(address) - ); - try_or_fail!( - self.state.metadata_mut().gasometer.access_addresses( - self.precompiles.addresses().iter().copied() - ) - ); + try_or_fail!(self.state.metadata_mut().gasometer.access_address(caller)); + try_or_fail!(self.state.metadata_mut().gasometer.access_address(address)); + try_or_fail!(self + .state + .metadata_mut() + .gasometer + .access_addresses(self.precompiles.addresses().iter().copied())); event!(Create { caller, @@ -658,10 +655,27 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, } } - if let Some(ret) = self.precompiles.run(code_address, &input, Some(gas_limit), &context, &mut self.state, is_static) { + if let Some(ret) = self.precompiles.run( + code_address, + &input, + Some(gas_limit), + &context, + &mut self.state, + is_static, + ) { return match ret { - Ok(PrecompileOutput { exit_status, output, cost, logs }) => { - for Log { address, topics, data } in logs { + Ok(PrecompileOutput { + exit_status, + output, + cost, + logs, + }) => { + for Log { + address, + topics, + data, + } in logs + { match self.log(address, topics, data) { Ok(_) => continue, Err(error) => { @@ -673,12 +687,12 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, let _ = self.state.metadata_mut().gasometer.record_cost(cost); let _ = self.exit_substate(StackExitKind::Succeeded); Capture::Exit((ExitReason::Succeed(exit_status), output)) - }, + } Err(e) => { let _ = self.exit_substate(StackExitKind::Failed); Capture::Exit((ExitReason::Error(e), Vec::new())) - }, - } + } + }; } let mut runtime = Runtime::new(Rc::new(code), Rc::new(input), context, self.config); From 335e2d94f72ca4d30a70dcd37e634dbc56faadd7 Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Wed, 28 Jul 2021 12:04:33 +0000 Subject: [PATCH 25/38] Clippy fix --- gasometer/src/costs.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/gasometer/src/costs.rs b/gasometer/src/costs.rs index 9c08a1d2b..9ac1702a4 100644 --- a/gasometer/src/costs.rs +++ b/gasometer/src/costs.rs @@ -205,6 +205,7 @@ pub fn sload_cost(is_cold: bool, config: &Config) -> u64 { } } +#[allow(clippy::collapsible_else_if)] pub fn sstore_cost( original: H256, current: H256, @@ -222,10 +223,8 @@ pub fn sstore_cost( (config.gas_sload, config.gas_sstore_reset) }; let gas_cost = if config.sstore_gas_metering { - if config.sstore_revert_under_stipend { - if gas <= config.call_stipend { - return Err(ExitError::OutOfGas); - } + if config.sstore_revert_under_stipend && gas <= config.call_stipend { + return Err(ExitError::OutOfGas); } if new == current { From 9bee750378387dd8eaa7bb774547fa89f16d68be Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Wed, 28 Jul 2021 12:08:06 +0000 Subject: [PATCH 26/38] Add comment to memory.rs change --- core/src/memory.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/src/memory.rs b/core/src/memory.rs index 075f944eb..8c578c59b 100644 --- a/core/src/memory.rs +++ b/core/src/memory.rs @@ -141,8 +141,12 @@ impl Memory { len: U256, data: &[u8], ) -> Result<(), ExitFatal> { + // Needed to pass ethereum test defined in + // https://github.com/ethereum/tests/commit/17f7e7a6c64bb878c1b6af9dc8371b46c133e46d + // (regardless of other inputs, a zero-length copy is defined to be a no-op). + // TODO: refactor `set` and `copy_large` (see + // https://github.com/rust-blockchain/evm/pull/40#discussion_r677180794) if len.is_zero() { - // a zero-length copy is defined to be a no-op return Ok(()); } From 79d12b1355d066bcb2d49d46b40e203580700097 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Fri, 30 Jul 2021 17:07:20 +0700 Subject: [PATCH 27/38] get_accessed_addresses -> accessed_addresses Co-authored-by: Wei Tang --- gasometer/src/lib.rs | 4 ++-- src/executor/stack/mod.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 20d4e650c..d3f80739c 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -110,7 +110,7 @@ impl<'config> Gasometer<'config> { } } - pub fn get_accessed_addresses(&self) -> Option> { + pub fn accessed_addresses(&self) -> Option> { self.inner .as_ref() .ok() @@ -118,7 +118,7 @@ impl<'config> Gasometer<'config> { .map(|set| set.iter()) } - pub fn get_accessed_storages( + pub fn accessed_storages( &self, ) -> Option> { self.inner diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 7295f600b..95845fb2e 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -38,10 +38,10 @@ impl<'config> StackSubstateMetadata<'config> { self.gasometer.record_stipend(other.gasometer.gas())?; self.gasometer .record_refund(other.gasometer.refunded_gas())?; - if let Some(addresses) = other.gasometer.get_accessed_addresses() { + if let Some(addresses) = other.gasometer.accessed_addresses() { self.gasometer.access_addresses(addresses.copied())?; } - if let Some(storages) = other.gasometer.get_accessed_storages() { + if let Some(storages) = other.gasometer.accessed_storages() { self.gasometer.access_storages(storages.copied())?; } From d16bdecc64145ee6757652b0ea8ebf893ba94c4d Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Fri, 30 Jul 2021 17:49:09 +0700 Subject: [PATCH 28/38] Don't imply use of a single state for precompiles --- src/executor/stack/mod.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 95845fb2e..eff48a273 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -96,9 +96,9 @@ pub struct PrecompileOutput { } /// Precompiles trait which allows for the execution of a precompile. -pub trait Precompiles { +pub trait Precompiles { /// Runs the precompile at the given address with input and context. - fn run( + fn run( &self, address: H160, input: &[u8], @@ -117,8 +117,8 @@ pub struct NoPrecompile { addresses: Vec, } -impl Precompiles for NoPrecompile { - fn run( +impl Precompiles for NoPrecompile { + fn run( &self, _address: H160, _input: &[u8], @@ -136,7 +136,7 @@ impl Precompiles for NoPrecompile { } /// Stack-based executor. -pub struct StackExecutor<'config, S: StackState<'config>, P: Precompiles> { +pub struct StackExecutor<'config, S, P: Precompiles> { config: &'config Config, precompiles: P, state: S, @@ -149,7 +149,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S, NoPrecompile> { } } -impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, P> { +impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, P> { /// Return a reference of the Config. pub fn config(&self) -> &'config Config { self.config @@ -723,7 +723,7 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, } } -impl<'config, S: StackState<'config>, P: Precompiles> Handler for StackExecutor<'config, S, P> { +impl<'config, S: StackState<'config>, P: Precompiles> Handler for StackExecutor<'config, S, P> { type CreateInterrupt = Infallible; type CreateFeedback = Infallible; type CallInterrupt = Infallible; From c2af4516d210b7520f6cb1c4c5d2d16b6a9d6103 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Mon, 2 Aug 2021 23:55:01 +0700 Subject: [PATCH 29/38] Move access lists to metadata --- gasometer/src/lib.rs | 379 +++++++++++++++++------------------- runtime/src/handler.rs | 7 + src/executor/stack/mod.rs | 147 +++++++++++--- src/executor/stack/state.rs | 10 + 4 files changed, 307 insertions(+), 236 deletions(-) diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index d3f80739c..36f20c525 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -28,7 +28,6 @@ mod memory; mod utils; use alloc::boxed::Box; -use alloc::collections::BTreeSet; use alloc::vec::Vec; use core::cmp::max; use evm_core::{ExitError, Opcode, Stack}; @@ -66,11 +65,11 @@ pub struct Gasometer<'config> { impl<'config> Gasometer<'config> { /// Create a new gasometer with given gas limit and config. pub fn new(gas_limit: u64, config: &'config Config) -> Self { - let (accessed_addresses, accessed_storage_keys) = if config.increase_state_access_gas { - (Some(BTreeSet::new()), Some(BTreeSet::new())) - } else { - (None, None) - }; + // let (accessed_addresses, accessed_storage_keys) = if config.increase_state_access_gas { + // (Some(BTreeSet::new()), Some(BTreeSet::new())) + // } else { + // (None, None) + // }; Self { gas_limit, @@ -80,19 +79,17 @@ impl<'config> Gasometer<'config> { used_gas: 0, refunded_gas: 0, config, - accessed_addresses, - accessed_storage_keys, parent: None, }), } } pub fn spawn(self, gas_limit: u64) -> Self { - let (accessed_addresses, accessed_storage_keys) = if self.config.increase_state_access_gas { - (Some(BTreeSet::new()), Some(BTreeSet::new())) - } else { - (None, None) - }; + // let (accessed_addresses, accessed_storage_keys) = if self.config.increase_state_access_gas { + // (Some(BTreeSet::new()), Some(BTreeSet::new())) + // } else { + // (None, None) + // }; let config = self.config; Self { @@ -103,30 +100,28 @@ impl<'config> Gasometer<'config> { used_gas: 0, refunded_gas: 0, config, - accessed_addresses, - accessed_storage_keys, parent: self.inner.ok().map(Box::new), }), } } - pub fn accessed_addresses(&self) -> Option> { - self.inner - .as_ref() - .ok() - .and_then(|inner| inner.accessed_addresses.as_ref()) - .map(|set| set.iter()) - } - - pub fn accessed_storages( - &self, - ) -> Option> { - self.inner - .as_ref() - .ok() - .and_then(|inner| inner.accessed_storage_keys.as_ref()) - .map(|set| set.iter()) - } + // pub fn accessed_addresses(&self) -> Option> { + // self.inner + // .as_ref() + // .ok() + // .and_then(|inner| inner.accessed_addresses.as_ref()) + // .map(|set| set.iter()) + // } + // + // pub fn accessed_storages( + // &self, + // ) -> Option> { + // self.inner + // .as_ref() + // .ok() + // .and_then(|inner| inner.accessed_storage_keys.as_ref()) + // .map(|set| set.iter()) + // } #[inline] /// Returns the numerical gas cost value. @@ -268,41 +263,41 @@ impl<'config> Gasometer<'config> { self.inner_mut()?.used_gas -= stipend; Ok(()) } - - /// Access an address (makes certain gas costs cheaper in the future) - pub fn access_address(&mut self, address: H160) -> Result<(), ExitError> { - let inner = self.inner_mut()?; - if let Some(accessed_addresses) = &mut inner.accessed_addresses { - accessed_addresses.insert(address); - } - Ok(()) - } - - pub fn access_addresses(&mut self, addresses: I) -> Result<(), ExitError> - where - I: Iterator, - { - let inner = self.inner_mut()?; - if let Some(accessed_addresses) = &mut inner.accessed_addresses { - for address in addresses { - accessed_addresses.insert(address); - } - } - Ok(()) - } - - pub fn access_storages(&mut self, iter: I) -> Result<(), ExitError> - where - I: Iterator, - { - let inner = self.inner_mut()?; - if let Some(accessed_storage_keys) = &mut inner.accessed_storage_keys { - for (address, key) in iter { - accessed_storage_keys.insert((address, key)); - } - } - Ok(()) - } + // + // /// Access an address (makes certain gas costs cheaper in the future) + // pub fn access_address(&mut self, address: H160) -> Result<(), ExitError> { + // let inner = self.inner_mut()?; + // if let Some(accessed_addresses) = &mut inner.accessed_addresses { + // accessed_addresses.insert(address); + // } + // Ok(()) + // } + // + // pub fn access_addresses(&mut self, addresses: I) -> Result<(), ExitError> + // where + // I: Iterator, + // { + // let inner = self.inner_mut()?; + // if let Some(accessed_addresses) = &mut inner.accessed_addresses { + // for address in addresses { + // accessed_addresses.insert(address); + // } + // } + // Ok(()) + // } + // + // pub fn access_storages(&mut self, iter: I) -> Result<(), ExitError> + // where + // I: Iterator, + // { + // let inner = self.inner_mut()?; + // if let Some(accessed_storage_keys) = &mut inner.accessed_storage_keys { + // for (address, key) in iter { + // accessed_storage_keys.insert((address, key)); + // } + // } + // Ok(()) + // } /// Record transaction cost. pub fn record_transaction(&mut self, cost: TransactionCost) -> Result<(), ExitError> { @@ -545,15 +540,15 @@ pub fn dynamic_opcode_cost( Opcode::SELFBALANCE => GasCost::Invalid, Opcode::EXTCODESIZE => GasCost::ExtCodeSize { - address: stack.peek(0)?.into(), + target_is_cold: handler.is_cold(stack.peek(0)?.into(), None), }, Opcode::BALANCE => GasCost::Balance { - address: stack.peek(0)?.into(), + target_is_cold: handler.is_cold(stack.peek(0)?.into(), None), }, Opcode::BLOCKHASH => GasCost::BlockHash, Opcode::EXTCODEHASH if config.has_ext_code_hash => GasCost::ExtCodeHash { - address: stack.peek(0)?.into(), + target_is_cold: handler.is_cold(stack.peek(0)?.into(), None), }, Opcode::EXTCODEHASH => GasCost::Invalid, @@ -562,7 +557,7 @@ pub fn dynamic_opcode_cost( GasCost::CallCode { value: U256::from_big_endian(&stack.peek(2)?[..]), gas: U256::from_big_endian(&stack.peek(0)?[..]), - target, + target_is_cold: handler.is_cold(target, None), target_exists: handler.exists(target), } } @@ -570,7 +565,7 @@ pub fn dynamic_opcode_cost( let target = stack.peek(1)?.into(); GasCost::StaticCall { gas: U256::from_big_endian(&stack.peek(0)?[..]), - target, + target_is_cold: handler.is_cold(target, None), target_exists: handler.exists(target), } } @@ -578,7 +573,7 @@ pub fn dynamic_opcode_cost( len: U256::from_big_endian(&stack.peek(1)?[..]), }, Opcode::EXTCODECOPY => GasCost::ExtCodeCopy { - address: stack.peek(0)?.into(), + target_is_cold: handler.is_cold(stack.peek(0)?.into(), None), len: U256::from_big_endian(&stack.peek(3)?[..]), }, Opcode::CALLDATACOPY | Opcode::CODECOPY => GasCost::VeryLowCopy { @@ -587,16 +582,18 @@ pub fn dynamic_opcode_cost( Opcode::EXP => GasCost::Exp { power: U256::from_big_endian(&stack.peek(1)?[..]), }, - Opcode::SLOAD => GasCost::SLoad { - address, - key: stack.peek(0)?, - }, + Opcode::SLOAD => { + let index = stack.peek(0)?; + GasCost::SLoad { + target_is_cold: handler.is_cold(address, Some(index)), + } + } Opcode::DELEGATECALL if config.has_delegate_call => { let target = stack.peek(1)?.into(); GasCost::DelegateCall { gas: U256::from_big_endian(&stack.peek(0)?[..]), - target, + target_is_cold: handler.is_cold(target, None), target_exists: handler.exists(target), } } @@ -613,11 +610,10 @@ pub fn dynamic_opcode_cost( let value = stack.peek(1)?; GasCost::SStore { - address, - key: index, original: handler.original_storage(address, index), current: handler.storage(address, index), new: value, + target_is_cold: handler.is_cold(address, Some(index)), } } Opcode::LOG0 if !is_static => GasCost::Log { @@ -648,7 +644,7 @@ pub fn dynamic_opcode_cost( let target = stack.peek(0)?.into(); GasCost::Suicide { value: handler.balance(address), - target, + target_is_cold: handler.is_cold(target, None), target_exists: handler.exists(target), already_removed: handler.deleted(address), } @@ -661,7 +657,7 @@ pub fn dynamic_opcode_cost( GasCost::Call { value: U256::from_big_endian(&stack.peek(2)?[..]), gas: U256::from_big_endian(&stack.peek(0)?[..]), - target, + target_is_cold: handler.is_cold(target, None), target_exists: handler.exists(target), } } @@ -742,8 +738,6 @@ struct Inner<'config> { used_gas: u64, refunded_gas: i64, config: &'config Config, - accessed_addresses: Option>, - accessed_storage_keys: Option>, parent: Option>>, } @@ -786,12 +780,12 @@ impl<'config> Inner<'config> { Ok(match cost { GasCost::Call { value, - target, + target_is_cold, target_exists, .. } => costs::call_cost( value, - self.check_and_update_address_cold(target), + target_is_cold, true, true, !target_exists, @@ -799,36 +793,36 @@ impl<'config> Inner<'config> { ), GasCost::CallCode { value, - target, + target_is_cold, target_exists, .. } => costs::call_cost( value, - self.check_and_update_address_cold(target), + target_is_cold, true, false, !target_exists, self.config, ), GasCost::DelegateCall { - target, + target_is_cold, target_exists, .. } => costs::call_cost( U256::zero(), - self.check_and_update_address_cold(target), + target_is_cold, false, false, !target_exists, self.config, ), GasCost::StaticCall { - target, + target_is_cold, target_exists, .. } => costs::call_cost( U256::zero(), - self.check_and_update_address_cold(target), + target_is_cold, false, true, !target_exists, @@ -837,30 +831,17 @@ impl<'config> Inner<'config> { GasCost::Suicide { value, - target, + target_is_cold, target_exists, .. - } => costs::suicide_cost( - value, - self.check_and_update_address_cold(target), - target_exists, - self.config, - ), + } => costs::suicide_cost(value, target_is_cold, target_exists, self.config), GasCost::SStore { .. } if self.config.estimate => self.config.gas_sstore_set, GasCost::SStore { - address, - key, original, current, new, - } => costs::sstore_cost( - original, - current, - new, - gas, - self.check_and_update_storage_cold(address, key), - self.config, - )?, + target_is_cold, + } => costs::sstore_cost(original, current, new, gas, target_is_cold, self.config)?, GasCost::Sha3 { len } => costs::sha3_cost(len)?, GasCost::Log { n, len } => costs::log_cost(n, len)?, @@ -868,10 +849,7 @@ impl<'config> Inner<'config> { GasCost::Exp { power } => costs::exp_cost(power, self.config)?, GasCost::Create => consts::G_CREATE, GasCost::Create2 { len } => costs::create2_cost(len)?, - GasCost::SLoad { address, key } => costs::sload_cost( - self.check_and_update_storage_cold(address, key), - self.config, - ), + GasCost::SLoad { target_is_cold } => costs::sload_cost(target_is_cold, self.config), GasCost::Zero => consts::G_ZERO, GasCost::Base => consts::G_BASE, @@ -879,80 +857,75 @@ impl<'config> Inner<'config> { GasCost::Low => consts::G_LOW, GasCost::Invalid => return Err(ExitError::OutOfGas), - GasCost::ExtCodeSize { address } => costs::address_access_cost( - self.check_and_update_address_cold(address), - self.config.gas_ext_code, - self.config, - ), - GasCost::ExtCodeCopy { address, len } => costs::extcodecopy_cost( + GasCost::ExtCodeSize { target_is_cold } => { + costs::address_access_cost(target_is_cold, self.config.gas_ext_code, self.config) + } + GasCost::ExtCodeCopy { + target_is_cold, len, - self.check_and_update_address_cold(address), - self.config, - )?, - GasCost::Balance { address } => costs::address_access_cost( - self.check_and_update_address_cold(address), - self.config.gas_balance, - self.config, - ), + } => costs::extcodecopy_cost(len, target_is_cold, self.config)?, + GasCost::Balance { target_is_cold } => { + costs::address_access_cost(target_is_cold, self.config.gas_balance, self.config) + } GasCost::BlockHash => consts::G_BLOCKHASH, - GasCost::ExtCodeHash { address } => costs::address_access_cost( - self.check_and_update_address_cold(address), + GasCost::ExtCodeHash { target_is_cold } => costs::address_access_cost( + target_is_cold, self.config.gas_ext_code_hash, self.config, ), }) } - fn check_and_update_address_cold(&mut self, address: H160) -> bool { - let is_cold = self.is_address_cold(&address); - if is_cold { - self.accessed_addresses - .as_mut() - .map(|set| set.insert(address)); - } - is_cold - } - - fn check_and_update_storage_cold(&mut self, address: H160, key: H256) -> bool { - let tuple = (address, key); - let is_cold = self.is_storage_cold(&tuple); - if is_cold { - self.accessed_storage_keys - .as_mut() - .map(|set| set.insert(tuple)); - } - is_cold - } - - fn is_address_cold(&self, address: &H160) -> bool { - if let Some(accessed_addresses) = &self.accessed_addresses { - if accessed_addresses.contains(address) { - false - } else { - self.parent - .as_ref() - .map(|p| p.is_address_cold(address)) - .unwrap_or(true) - } - } else { - false - } - } - - fn is_storage_cold(&self, tuple: &(H160, H256)) -> bool { - if let Some(accessed_storage_keys) = &self.accessed_storage_keys { - if accessed_storage_keys.contains(tuple) { - false - } else { - self.parent - .as_ref() - .map(|p| p.is_storage_cold(tuple)) - .unwrap_or(true) - } - } else { - false - } - } + // fn check_and_update_address_cold(&mut self, address: H160) -> bool { + // let is_cold = self.is_address_cold(&address); + // if is_cold { + // self.accessed_addresses + // .as_mut() + // .map(|set| set.insert(address)); + // } + // is_cold + // } + // + // fn check_and_update_storage_cold(&mut self, address: H160, key: H256) -> bool { + // let tuple = (address, key); + // let is_cold = self.is_storage_cold(&tuple); + // if is_cold { + // self.accessed_storage_keys + // .as_mut() + // .map(|set| set.insert(tuple)); + // } + // is_cold + // } + // + // fn is_address_cold(&self, address: &H160) -> bool { + // if let Some(accessed_addresses) = &self.accessed_addresses { + // if accessed_addresses.contains(address) { + // false + // } else { + // self.parent + // .as_ref() + // .map(|p| p.is_address_cold(address)) + // .unwrap_or(true) + // } + // } else { + // false + // } + // } + // + // fn is_storage_cold(&self, tuple: &(H160, H256)) -> bool { + // if let Some(accessed_storage_keys) = &self.accessed_storage_keys { + // if accessed_storage_keys.contains(tuple) { + // false + // } else { + // self.parent + // .as_ref() + // .map(|p| p.is_storage_cold(tuple)) + // .unwrap_or(true) + // } + // } else { + // false + // } + // } fn gas_refund(&self, cost: GasCost) -> i64 { match cost { @@ -988,20 +961,20 @@ pub enum GasCost { /// Gas cost for `EXTCODESIZE`. ExtCodeSize { - /// Address to get code size of - address: H160, + /// True if address has not been previously accessed in this transaction + target_is_cold: bool, }, /// Gas cost for `BALANCE`. Balance { - /// Address to get balance of - address: H160, + /// True if address has not been previously accessed in this transaction + target_is_cold: bool, }, /// Gas cost for `BLOCKHASH`. BlockHash, /// Gas cost for `EXTBLOCKHASH`. ExtCodeHash { - /// Address to get code hash of - address: H160, + /// True if address has not been previously accessed in this transaction + target_is_cold: bool, }, /// Gas cost for `CALL`. @@ -1010,8 +983,8 @@ pub enum GasCost { value: U256, /// Call gas. gas: U256, - /// Address to call - target: H160, + /// True if target has not been previously accessed in this transaction + target_is_cold: bool, /// Whether the target exists. target_exists: bool, }, @@ -1021,8 +994,8 @@ pub enum GasCost { value: U256, /// Call gas. gas: U256, - /// Address to call - target: H160, + /// True if target has not been previously accessed in this transaction + target_is_cold: bool, /// Whether the target exists. target_exists: bool, }, @@ -1030,8 +1003,8 @@ pub enum GasCost { DelegateCall { /// Call gas. gas: U256, - /// Address to call - target: H160, + /// True if target has not been previously accessed in this transaction + target_is_cold: bool, /// Whether the target exists. target_exists: bool, }, @@ -1039,8 +1012,8 @@ pub enum GasCost { StaticCall { /// Call gas. gas: U256, - /// Address to call - target: H160, + /// True if target has not been previously accessed in this transaction + target_is_cold: bool, /// Whether the target exists. target_exists: bool, }, @@ -1048,8 +1021,8 @@ pub enum GasCost { Suicide { /// Value. value: U256, - /// ETH recipient - target: H160, + /// True if target has not been previously accessed in this transaction + target_is_cold: bool, /// Whether the target exists. target_exists: bool, /// Whether the target has already been removed. @@ -1057,16 +1030,14 @@ pub enum GasCost { }, /// Gas cost for `SSTORE`. SStore { - /// Address the storage is for - address: H160, - /// Key - key: H256, /// Original value. original: H256, /// Current value. current: H256, /// New value. new: H256, + /// True if target has not been previously accessed in this transaction + target_is_cold: bool, }, /// Gas cost for `SHA3`. Sha3 { @@ -1082,8 +1053,8 @@ pub enum GasCost { }, /// Gas cost for `EXTCODECOPY`. ExtCodeCopy { - /// Address to copy code from - address: H160, + /// True if target has not been previously accessed in this transaction + target_is_cold: bool, /// Length. len: U256, }, @@ -1106,10 +1077,8 @@ pub enum GasCost { }, /// Gas cost for `SLOAD`. SLoad { - /// Address the storage is for - address: H160, - /// Key to read - key: H256, + /// True if target has not been previously accessed in this transaction + target_is_cold: bool, }, } diff --git a/runtime/src/handler.rs b/runtime/src/handler.rs index 803ef9abd..e7cec46c7 100644 --- a/runtime/src/handler.rs +++ b/runtime/src/handler.rs @@ -62,6 +62,13 @@ pub trait Handler { fn exists(&self, address: H160) -> bool; /// Check whether an address has already been deleted. fn deleted(&self, address: H160) -> bool; + /// Checks if the address or (address, index) pair has been previously accessed + /// (or set in `accessed_addresses` / `accessed_storage_keys` via an access list + /// transaction). + /// References: + /// * https://eips.ethereum.org/EIPS/eip-2929 + /// * https://eips.ethereum.org/EIPS/eip-2930 + fn is_cold(&self, address: H160, index: Option) -> bool; /// Set storage value of address at index. fn set_storage(&mut self, address: H160, index: H256, value: H256) -> Result<(), ExitError>; diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index eff48a273..bcfce1561 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -7,7 +7,7 @@ use crate::{ Capture, Config, Context, CreateScheme, ExitError, ExitReason, ExitSucceed, Handler, Opcode, Runtime, Stack, Transfer, }; -use alloc::{rc::Rc, vec::Vec}; +use alloc::{collections::BTreeSet, rc::Rc, vec::Vec}; use core::{cmp::min, convert::Infallible}; use ethereum::Log; use primitive_types::{H160, H256, U256}; @@ -19,18 +19,63 @@ pub enum StackExitKind { Failed, } +#[derive(Default)] +struct Accessed { + accessed_addresses: BTreeSet, + accessed_storage: BTreeSet<(H160, H256)>, +} + +impl Accessed { + fn access_address(&mut self, address: H160) { + self.accessed_addresses.insert(address); + } + + fn access_addresses(&mut self, addresses: I) + where + I: Iterator, + { + for address in addresses { + self.accessed_addresses.insert(address); + } + } + + fn access_storages(&mut self, storages: I) + where + I: Iterator, + { + for storage in storages { + self.accessed_storage.insert((storage.0, storage.1)); + } + } + + fn accessed_addresses(&self) -> alloc::collections::btree_set::Iter<'_, H160> { + self.accessed_addresses.iter() + } + + fn accessed_storages(&self) -> alloc::collections::btree_set::Iter<'_, (H160, H256)> { + self.accessed_storage.iter() + } +} + pub struct StackSubstateMetadata<'config> { gasometer: Gasometer<'config>, is_static: bool, depth: Option, + accessed: Option, } impl<'config> StackSubstateMetadata<'config> { pub fn new(gas_limit: u64, config: &'config Config) -> Self { + let accessed = if config.increase_state_access_gas { + Some(Accessed::default()) + } else { + None + }; Self { gasometer: Gasometer::new(gas_limit, config), is_static: false, depth: None, + accessed, } } @@ -38,12 +83,16 @@ impl<'config> StackSubstateMetadata<'config> { self.gasometer.record_stipend(other.gasometer.gas())?; self.gasometer .record_refund(other.gasometer.refunded_gas())?; - if let Some(addresses) = other.gasometer.accessed_addresses() { - self.gasometer.access_addresses(addresses.copied())?; - } - if let Some(storages) = other.gasometer.accessed_storages() { - self.gasometer.access_storages(storages.copied())?; - } + other.accessed_addresses().map(|addresses| { + self.accessed + .as_mut() + .map(|accessed| accessed.access_addresses(addresses.copied())); + }); + other.accessed_storages().map(|storages| { + self.accessed + .as_mut() + .map(|accessed| accessed.access_storages(storages.copied())); + }); Ok(()) } @@ -67,6 +116,7 @@ impl<'config> StackSubstateMetadata<'config> { None => Some(0), Some(n) => Some(n + 1), }, + accessed: Default::default(), } } @@ -85,6 +135,47 @@ impl<'config> StackSubstateMetadata<'config> { pub fn depth(&self) -> Option { self.depth } + + fn access_address(&mut self, address: H160) { + debug_assert!(self.accessed.is_some()); + self.accessed + .as_mut() + .map(|accessed| accessed.access_address(address)); + } + + fn access_addresses(&mut self, addresses: I) + where + I: Iterator, + { + debug_assert!(self.accessed.is_some()); + self.accessed + .as_mut() + .map(|accessed| accessed.access_addresses(addresses)); + } + + fn access_storages(&mut self, storages: I) + where + I: Iterator, + { + debug_assert!(self.accessed.is_some()); + self.accessed + .as_mut() + .map(|accessed| accessed.access_storages(storages)); + } + + fn accessed_addresses(&self) -> Option> { + debug_assert!(self.accessed.is_some()); + self.accessed + .as_ref() + .map(|accessed| accessed.accessed_addresses()) + } + + fn accessed_storages(&self) -> Option> { + debug_assert!(self.accessed.is_some()); + self.accessed + .as_ref() + .map(|accessed| accessed.accessed_storages()) + } } #[derive(Debug, Eq, PartialEq, Clone)] @@ -219,9 +310,7 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, Err(e) => return e.into(), } - if let Err(e) = Self::initialize_with_access_list(gasometer, access_list) { - return e.into(); - } + self.initialize_with_access_list(access_list); match self.create_inner( caller, @@ -254,9 +343,7 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, } let code_hash = H256::from_slice(Keccak256::digest(&init_code).as_slice()); - if let Err(e) = Self::initialize_with_access_list(gasometer, access_list) { - return e.into(); - } + self.initialize_with_access_list(access_list); match self.create_inner( caller, @@ -307,13 +394,9 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, .copied() .chain(core::iter::once(caller)) .chain(core::iter::once(address)); - if let Err(e) = gasometer.access_addresses(addresses) { - return (e.into(), Vec::new()); - } + self.state.metadata_mut().access_addresses(addresses); - if let Err(e) = Self::initialize_with_access_list(gasometer, access_list) { - return (e.into(), Vec::new()); - } + self.initialize_with_access_list(access_list); } self.state.inc_nonce(caller); @@ -389,17 +472,14 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, } } - fn initialize_with_access_list( - gasometer: &mut Gasometer, - access_list: Vec<(H160, Vec)>, - ) -> Result<(), ExitError> { + fn initialize_with_access_list(&mut self, access_list: Vec<(H160, Vec)>) { let addresses = access_list.iter().map(|a| a.0); - gasometer.access_addresses(addresses)?; + self.state.metadata_mut().access_addresses(addresses); let storage_keys = access_list .into_iter() .flat_map(|(address, keys)| keys.into_iter().map(move |key| (address, key))); - gasometer.access_storages(storage_keys) + self.state.metadata_mut().access_storages(storage_keys); } fn create_inner( @@ -426,13 +506,11 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, let address = self.create_address(scheme); - try_or_fail!(self.state.metadata_mut().gasometer.access_address(caller)); - try_or_fail!(self.state.metadata_mut().gasometer.access_address(address)); - try_or_fail!(self - .state + self.state.metadata_mut().access_address(caller); + self.state.metadata_mut().access_address(address); + self.state .metadata_mut() - .gasometer - .access_addresses(self.precompiles.addresses().iter().copied())); + .access_addresses(self.precompiles.addresses().iter().copied()); event!(Create { caller, @@ -767,6 +845,13 @@ impl<'config, S: StackState<'config>, P: Precompiles> Handler for StackExecutor< } } + fn is_cold(&self, address: H160, maybe_index: Option) -> bool { + match maybe_index { + None => self.state.is_known(address), + Some(index) => self.state.is_storage_known(address, index), + } + } + fn gas_left(&self) -> U256 { U256::from(self.state.metadata().gasometer.gas()) } diff --git a/src/executor/stack/state.rs b/src/executor/stack/state.rs index 9b476fdd2..2dbd549b6 100644 --- a/src/executor/stack/state.rs +++ b/src/executor/stack/state.rs @@ -375,6 +375,8 @@ pub trait StackState<'config>: Backend { fn is_empty(&self, address: H160) -> bool; fn deleted(&self, address: H160) -> bool; + fn is_known(&self, address: H160) -> bool; + fn is_storage_known(&self, address: H160, key: H256) -> bool; fn inc_nonce(&mut self, address: H160); fn set_storage(&mut self, address: H160, key: H256, value: H256); @@ -491,6 +493,14 @@ impl<'backend, 'config, B: Backend> StackState<'config> for MemoryStackState<'ba self.substate.deleted(address) } + fn is_known(&self, address: H160) -> bool { + self.substate.known_account(address).is_some() + } + + fn is_storage_known(&self, address: H160, key: H256) -> bool { + self.substate.known_storage(address, key).is_some() + } + fn inc_nonce(&mut self, address: H160) { self.substate.inc_nonce(address, self.backend); } From 6b46fa8795d13454509e5168bdcf00b17f5be3d8 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Tue, 3 Aug 2021 00:05:25 +0700 Subject: [PATCH 30/38] Handle all clippy errors --- core/src/eval/bitwise.rs | 4 ++-- core/src/memory.rs | 2 +- core/src/utils.rs | 32 ++++++++++++++++---------------- src/executor/stack/mod.rs | 36 ++++++++++++++++-------------------- src/executor/stack/state.rs | 2 +- 5 files changed, 36 insertions(+), 40 deletions(-) diff --git a/core/src/eval/bitwise.rs b/core/src/eval/bitwise.rs index 3d0dd19b3..5667865a0 100644 --- a/core/src/eval/bitwise.rs +++ b/core/src/eval/bitwise.rs @@ -84,7 +84,7 @@ pub fn sar(shift: U256, value: U256) -> U256 { let I256(sign, _) = value; match sign { // value is 0 or >=1, pushing 0 - Sign::Plus | Sign::NoSign => U256::zero(), + Sign::Plus | Sign::No => U256::zero(), // value is <0, pushing -1 Sign::Minus => I256(Sign::Minus, U256::one()).into(), } @@ -92,7 +92,7 @@ pub fn sar(shift: U256, value: U256) -> U256 { let shift: u64 = shift.as_u64(); match value.0 { - Sign::Plus | Sign::NoSign => value.1 >> shift as usize, + Sign::Plus | Sign::No => value.1 >> shift as usize, Sign::Minus => { let shifted = ((value.1.overflowing_sub(U256::one()).0) >> shift as usize) .overflowing_add(U256::one()) diff --git a/core/src/memory.rs b/core/src/memory.rs index 8c578c59b..7bb09629b 100644 --- a/core/src/memory.rs +++ b/core/src/memory.rs @@ -122,7 +122,7 @@ impl Memory { } if target_size > value.len() { - self.data[offset..((value.len()) + offset)].clone_from_slice(&value); + self.data[offset..((value.len()) + offset)].clone_from_slice(value); for index in (value.len())..target_size { self.data[offset + index] = 0; } diff --git a/core/src/utils.rs b/core/src/utils.rs index c78dd49e6..104531cb0 100644 --- a/core/src/utils.rs +++ b/core/src/utils.rs @@ -6,7 +6,7 @@ use primitive_types::U256; pub enum Sign { Plus, Minus, - NoSign, + No, } const SIGN_BIT_MASK: U256 = U256([ @@ -22,7 +22,7 @@ pub struct I256(pub Sign, pub U256); impl I256 { /// Zero value of I256. pub fn zero() -> I256 { - I256(Sign::NoSign, U256::zero()) + I256(Sign::No, U256::zero()) } /// Minimum value of I256. pub fn min_value() -> I256 { @@ -33,14 +33,14 @@ impl I256 { impl Ord for I256 { fn cmp(&self, other: &I256) -> Ordering { match (self.0, other.0) { - (Sign::NoSign, Sign::NoSign) => Ordering::Equal, - (Sign::NoSign, Sign::Plus) => Ordering::Less, - (Sign::NoSign, Sign::Minus) => Ordering::Greater, - (Sign::Minus, Sign::NoSign) => Ordering::Less, + (Sign::No, Sign::No) => Ordering::Equal, + (Sign::No, Sign::Plus) => Ordering::Less, + (Sign::No, Sign::Minus) => Ordering::Greater, + (Sign::Minus, Sign::No) => Ordering::Less, (Sign::Minus, Sign::Plus) => Ordering::Less, (Sign::Minus, Sign::Minus) => self.1.cmp(&other.1).reverse(), (Sign::Plus, Sign::Minus) => Ordering::Greater, - (Sign::Plus, Sign::NoSign) => Ordering::Greater, + (Sign::Plus, Sign::No) => Ordering::Greater, (Sign::Plus, Sign::Plus) => self.1.cmp(&other.1), } } @@ -73,7 +73,7 @@ impl From for I256 { impl From for U256 { fn from(value: I256) -> U256 { let sign = value.0; - if sign == Sign::NoSign { + if sign == Sign::No { U256::zero() } else if sign == Sign::Plus { value.1 @@ -102,14 +102,14 @@ impl Div for I256 { } match (self.0, other.0) { - (Sign::NoSign, Sign::Plus) - | (Sign::Plus, Sign::NoSign) - | (Sign::NoSign, Sign::NoSign) + (Sign::No, Sign::Plus) + | (Sign::Plus, Sign::No) + | (Sign::No, Sign::No) | (Sign::Plus, Sign::Plus) | (Sign::Minus, Sign::Minus) => I256(Sign::Plus, d), - (Sign::NoSign, Sign::Minus) + (Sign::No, Sign::Minus) | (Sign::Plus, Sign::Minus) - | (Sign::Minus, Sign::NoSign) + | (Sign::Minus, Sign::No) | (Sign::Minus, Sign::Plus) => I256(Sign::Minus, d), } } @@ -148,10 +148,10 @@ mod tests { assert_eq!(100i8 / 2, 50i8); // Now the same calculations based on i256 - let one = I256(Sign::NoSign, U256::from(1)); - let one_hundred = I256(Sign::NoSign, U256::from(100)); + let one = I256(Sign::No, U256::from(1)); + let one_hundred = I256(Sign::No, U256::from(100)); let fifty = I256(Sign::Plus, U256::from(50)); - let two = I256(Sign::NoSign, U256::from(2)); + let two = I256(Sign::No, U256::from(2)); let neg_one_hundred = I256(Sign::Minus, U256::from(100)); let minus_one = I256(Sign::Minus, U256::from(1)); let max_value = I256(Sign::Plus, U256::from(2).pow(U256::from(255)) - 1); diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index bcfce1561..cba814318 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -83,16 +83,12 @@ impl<'config> StackSubstateMetadata<'config> { self.gasometer.record_stipend(other.gasometer.gas())?; self.gasometer .record_refund(other.gasometer.refunded_gas())?; - other.accessed_addresses().map(|addresses| { - self.accessed - .as_mut() - .map(|accessed| accessed.access_addresses(addresses.copied())); - }); - other.accessed_storages().map(|storages| { - self.accessed - .as_mut() - .map(|accessed| accessed.access_storages(storages.copied())); - }); + if let Some(addresses) = other.accessed_addresses() { + self.access_addresses(addresses.copied()); + }; + if let Some(storages) = other.accessed_storages() { + self.access_storages(storages.copied()); + }; Ok(()) } @@ -138,9 +134,9 @@ impl<'config> StackSubstateMetadata<'config> { fn access_address(&mut self, address: H160) { debug_assert!(self.accessed.is_some()); - self.accessed - .as_mut() - .map(|accessed| accessed.access_address(address)); + if let Some(accessed) = &mut self.accessed { + accessed.access_address(address) + } } fn access_addresses(&mut self, addresses: I) @@ -148,9 +144,9 @@ impl<'config> StackSubstateMetadata<'config> { I: Iterator, { debug_assert!(self.accessed.is_some()); - self.accessed - .as_mut() - .map(|accessed| accessed.access_addresses(addresses)); + if let Some(accessed) = &mut self.accessed { + accessed.access_addresses(addresses); + } } fn access_storages(&mut self, storages: I) @@ -158,9 +154,9 @@ impl<'config> StackSubstateMetadata<'config> { I: Iterator, { debug_assert!(self.accessed.is_some()); - self.accessed - .as_mut() - .map(|accessed| accessed.access_storages(storages)); + if let Some(accessed) = &mut self.accessed { + accessed.access_storages(storages); + } } fn accessed_addresses(&self) -> Option> { @@ -968,7 +964,7 @@ impl<'config, S: StackState<'config>, P: Precompiles> Handler for StackExecutor< opcode, stack, is_static, - &self.config, + self.config, self, )?; diff --git a/src/executor/stack/state.rs b/src/executor/stack/state.rs index 2dbd549b6..81ba27bb1 100644 --- a/src/executor/stack/state.rs +++ b/src/executor/stack/state.rs @@ -132,7 +132,7 @@ impl<'config> MemoryStackSubstate<'config> { } let mut reset_keys = BTreeSet::new(); for (address, key) in self.storages.keys() { - if resets.contains(&address) { + if resets.contains(address) { reset_keys.insert((*address, *key)); } } From 56a7ee056a29cb5445083165aa07e4348feaa3f7 Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Mon, 2 Aug 2021 17:57:04 +0000 Subject: [PATCH 31/38] Remove debug asserts (they do not passs Istanbul tests) --- src/executor/stack/mod.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index cba814318..7337cf4b5 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -133,7 +133,6 @@ impl<'config> StackSubstateMetadata<'config> { } fn access_address(&mut self, address: H160) { - debug_assert!(self.accessed.is_some()); if let Some(accessed) = &mut self.accessed { accessed.access_address(address) } @@ -143,7 +142,6 @@ impl<'config> StackSubstateMetadata<'config> { where I: Iterator, { - debug_assert!(self.accessed.is_some()); if let Some(accessed) = &mut self.accessed { accessed.access_addresses(addresses); } @@ -153,21 +151,18 @@ impl<'config> StackSubstateMetadata<'config> { where I: Iterator, { - debug_assert!(self.accessed.is_some()); if let Some(accessed) = &mut self.accessed { accessed.access_storages(storages); } } fn accessed_addresses(&self) -> Option> { - debug_assert!(self.accessed.is_some()); self.accessed .as_ref() .map(|accessed| accessed.accessed_addresses()) } fn accessed_storages(&self) -> Option> { - debug_assert!(self.accessed.is_some()); self.accessed .as_ref() .map(|accessed| accessed.accessed_storages()) From 2c924771b094b4c5cb9cde8f372ac2024d5216c8 Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Mon, 2 Aug 2021 18:00:27 +0000 Subject: [PATCH 32/38] Remove commented out code --- gasometer/src/lib.rs | 115 ------------------------------------------- 1 file changed, 115 deletions(-) diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 36f20c525..54cf041fe 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -65,12 +65,6 @@ pub struct Gasometer<'config> { impl<'config> Gasometer<'config> { /// Create a new gasometer with given gas limit and config. pub fn new(gas_limit: u64, config: &'config Config) -> Self { - // let (accessed_addresses, accessed_storage_keys) = if config.increase_state_access_gas { - // (Some(BTreeSet::new()), Some(BTreeSet::new())) - // } else { - // (None, None) - // }; - Self { gas_limit, config, @@ -85,11 +79,6 @@ impl<'config> Gasometer<'config> { } pub fn spawn(self, gas_limit: u64) -> Self { - // let (accessed_addresses, accessed_storage_keys) = if self.config.increase_state_access_gas { - // (Some(BTreeSet::new()), Some(BTreeSet::new())) - // } else { - // (None, None) - // }; let config = self.config; Self { @@ -105,24 +94,6 @@ impl<'config> Gasometer<'config> { } } - // pub fn accessed_addresses(&self) -> Option> { - // self.inner - // .as_ref() - // .ok() - // .and_then(|inner| inner.accessed_addresses.as_ref()) - // .map(|set| set.iter()) - // } - // - // pub fn accessed_storages( - // &self, - // ) -> Option> { - // self.inner - // .as_ref() - // .ok() - // .and_then(|inner| inner.accessed_storage_keys.as_ref()) - // .map(|set| set.iter()) - // } - #[inline] /// Returns the numerical gas cost value. pub fn gas_cost(&mut self, cost: GasCost, gas: u64) -> Result { @@ -263,41 +234,6 @@ impl<'config> Gasometer<'config> { self.inner_mut()?.used_gas -= stipend; Ok(()) } - // - // /// Access an address (makes certain gas costs cheaper in the future) - // pub fn access_address(&mut self, address: H160) -> Result<(), ExitError> { - // let inner = self.inner_mut()?; - // if let Some(accessed_addresses) = &mut inner.accessed_addresses { - // accessed_addresses.insert(address); - // } - // Ok(()) - // } - // - // pub fn access_addresses(&mut self, addresses: I) -> Result<(), ExitError> - // where - // I: Iterator, - // { - // let inner = self.inner_mut()?; - // if let Some(accessed_addresses) = &mut inner.accessed_addresses { - // for address in addresses { - // accessed_addresses.insert(address); - // } - // } - // Ok(()) - // } - // - // pub fn access_storages(&mut self, iter: I) -> Result<(), ExitError> - // where - // I: Iterator, - // { - // let inner = self.inner_mut()?; - // if let Some(accessed_storage_keys) = &mut inner.accessed_storage_keys { - // for (address, key) in iter { - // accessed_storage_keys.insert((address, key)); - // } - // } - // Ok(()) - // } /// Record transaction cost. pub fn record_transaction(&mut self, cost: TransactionCost) -> Result<(), ExitError> { @@ -876,57 +812,6 @@ impl<'config> Inner<'config> { }) } - // fn check_and_update_address_cold(&mut self, address: H160) -> bool { - // let is_cold = self.is_address_cold(&address); - // if is_cold { - // self.accessed_addresses - // .as_mut() - // .map(|set| set.insert(address)); - // } - // is_cold - // } - // - // fn check_and_update_storage_cold(&mut self, address: H160, key: H256) -> bool { - // let tuple = (address, key); - // let is_cold = self.is_storage_cold(&tuple); - // if is_cold { - // self.accessed_storage_keys - // .as_mut() - // .map(|set| set.insert(tuple)); - // } - // is_cold - // } - // - // fn is_address_cold(&self, address: &H160) -> bool { - // if let Some(accessed_addresses) = &self.accessed_addresses { - // if accessed_addresses.contains(address) { - // false - // } else { - // self.parent - // .as_ref() - // .map(|p| p.is_address_cold(address)) - // .unwrap_or(true) - // } - // } else { - // false - // } - // } - // - // fn is_storage_cold(&self, tuple: &(H160, H256)) -> bool { - // if let Some(accessed_storage_keys) = &self.accessed_storage_keys { - // if accessed_storage_keys.contains(tuple) { - // false - // } else { - // self.parent - // .as_ref() - // .map(|p| p.is_storage_cold(tuple)) - // .unwrap_or(true) - // } - // } else { - // false - // } - // } - fn gas_refund(&self, cost: GasCost) -> i64 { match cost { _ if self.config.estimate => 0, From 62c1c529ce0df3a24b3c0eec923a88a28a041adc Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Mon, 2 Aug 2021 18:03:19 +0000 Subject: [PATCH 33/38] Resore immutability of gas_cost calls --- gasometer/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 54cf041fe..c5a61ec0e 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -96,8 +96,8 @@ impl<'config> Gasometer<'config> { #[inline] /// Returns the numerical gas cost value. - pub fn gas_cost(&mut self, cost: GasCost, gas: u64) -> Result { - match self.inner.as_mut() { + pub fn gas_cost(&self, cost: GasCost, gas: u64) -> Result { + match self.inner.as_ref() { Ok(inner) => inner.gas_cost(cost, gas), Err(e) => Err(e.clone()), } @@ -712,7 +712,7 @@ impl<'config> Inner<'config> { } /// Returns the gas cost numerical value. - fn gas_cost(&mut self, cost: GasCost, gas: u64) -> Result { + fn gas_cost(&self, cost: GasCost, gas: u64) -> Result { Ok(match cost { GasCost::Call { value, From 0d706576ac393abd801d24ff87129cb9462b90f5 Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Mon, 2 Aug 2021 18:06:07 +0000 Subject: [PATCH 34/38] Remove obsolete additions made to gasometer --- gasometer/src/lib.rs | 19 ------------------- src/executor/stack/mod.rs | 3 +-- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index c5a61ec0e..4f7808460 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -27,7 +27,6 @@ mod costs; mod memory; mod utils; -use alloc::boxed::Box; use alloc::vec::Vec; use core::cmp::max; use evm_core::{ExitError, Opcode, Stack}; @@ -73,23 +72,6 @@ impl<'config> Gasometer<'config> { used_gas: 0, refunded_gas: 0, config, - parent: None, - }), - } - } - - pub fn spawn(self, gas_limit: u64) -> Self { - let config = self.config; - - Self { - gas_limit, - config, - inner: Ok(Inner { - memory_gas: 0, - used_gas: 0, - refunded_gas: 0, - config, - parent: self.inner.ok().map(Box::new), }), } } @@ -674,7 +656,6 @@ struct Inner<'config> { used_gas: u64, refunded_gas: i64, config: &'config Config, - parent: Option>>, } impl<'config> Inner<'config> { diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 7337cf4b5..8a20001c5 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -105,8 +105,7 @@ impl<'config> StackSubstateMetadata<'config> { pub fn spit_child(&self, gas_limit: u64, is_static: bool) -> Self { Self { - // TODO: should be able to get rid of this clone - gasometer: self.gasometer.clone().spawn(gas_limit), + gasometer: Gasometer::new(gas_limit, self.gasometer.config()), is_static: is_static || self.is_static, depth: match self.depth { None => Some(0), From 2a08e99bdeea60bae8a06e561becbf24e939da0d Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Mon, 2 Aug 2021 19:27:48 +0000 Subject: [PATCH 35/38] Fix storage access tracking and is_cold functionality --- gasometer/src/lib.rs | 65 ++++++++++++++++++++++++++++--------- src/executor/stack/mod.rs | 25 +++++++++++--- src/executor/stack/state.rs | 34 +++++++++++++++---- 3 files changed, 97 insertions(+), 27 deletions(-) diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 4f7808460..e22511090 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -439,7 +439,8 @@ pub fn dynamic_opcode_cost( is_static: bool, config: &Config, handler: &H, -) -> Result<(GasCost, Option), ExitError> { +) -> Result<(GasCost, StorageTarget, Option), ExitError> { + let mut storage_target = StorageTarget::None; let gas_cost = match opcode { Opcode::RETURN => GasCost::Zero, @@ -457,21 +458,34 @@ pub fn dynamic_opcode_cost( Opcode::SELFBALANCE if config.has_self_balance => GasCost::Low, Opcode::SELFBALANCE => GasCost::Invalid, - Opcode::EXTCODESIZE => GasCost::ExtCodeSize { - target_is_cold: handler.is_cold(stack.peek(0)?.into(), None), - }, - Opcode::BALANCE => GasCost::Balance { - target_is_cold: handler.is_cold(stack.peek(0)?.into(), None), - }, + Opcode::EXTCODESIZE => { + let target = stack.peek(0)?.into(); + storage_target = StorageTarget::Address(target); + GasCost::ExtCodeSize { + target_is_cold: handler.is_cold(target, None), + } + } + Opcode::BALANCE => { + let target = stack.peek(0)?.into(); + storage_target = StorageTarget::Address(target); + GasCost::Balance { + target_is_cold: handler.is_cold(target, None), + } + } Opcode::BLOCKHASH => GasCost::BlockHash, - Opcode::EXTCODEHASH if config.has_ext_code_hash => GasCost::ExtCodeHash { - target_is_cold: handler.is_cold(stack.peek(0)?.into(), None), - }, + Opcode::EXTCODEHASH if config.has_ext_code_hash => { + let target = stack.peek(0)?.into(); + storage_target = StorageTarget::Address(target); + GasCost::ExtCodeHash { + target_is_cold: handler.is_cold(target, None), + } + } Opcode::EXTCODEHASH => GasCost::Invalid, Opcode::CALLCODE => { let target = stack.peek(1)?.into(); + storage_target = StorageTarget::Address(target); GasCost::CallCode { value: U256::from_big_endian(&stack.peek(2)?[..]), gas: U256::from_big_endian(&stack.peek(0)?[..]), @@ -481,6 +495,7 @@ pub fn dynamic_opcode_cost( } Opcode::STATICCALL => { let target = stack.peek(1)?.into(); + storage_target = StorageTarget::Address(target); GasCost::StaticCall { gas: U256::from_big_endian(&stack.peek(0)?[..]), target_is_cold: handler.is_cold(target, None), @@ -490,10 +505,14 @@ pub fn dynamic_opcode_cost( Opcode::SHA3 => GasCost::Sha3 { len: U256::from_big_endian(&stack.peek(1)?[..]), }, - Opcode::EXTCODECOPY => GasCost::ExtCodeCopy { - target_is_cold: handler.is_cold(stack.peek(0)?.into(), None), - len: U256::from_big_endian(&stack.peek(3)?[..]), - }, + Opcode::EXTCODECOPY => { + let target = stack.peek(0)?.into(); + storage_target = StorageTarget::Address(target); + GasCost::ExtCodeCopy { + target_is_cold: handler.is_cold(target, None), + len: U256::from_big_endian(&stack.peek(3)?[..]), + } + } Opcode::CALLDATACOPY | Opcode::CODECOPY => GasCost::VeryLowCopy { len: U256::from_big_endian(&stack.peek(2)?[..]), }, @@ -502,6 +521,7 @@ pub fn dynamic_opcode_cost( }, Opcode::SLOAD => { let index = stack.peek(0)?; + storage_target = StorageTarget::Slot(address, index); GasCost::SLoad { target_is_cold: handler.is_cold(address, Some(index)), } @@ -509,6 +529,7 @@ pub fn dynamic_opcode_cost( Opcode::DELEGATECALL if config.has_delegate_call => { let target = stack.peek(1)?.into(); + storage_target = StorageTarget::Address(target); GasCost::DelegateCall { gas: U256::from_big_endian(&stack.peek(0)?[..]), target_is_cold: handler.is_cold(target, None), @@ -526,6 +547,7 @@ pub fn dynamic_opcode_cost( Opcode::SSTORE if !is_static => { let index = stack.peek(0)?; let value = stack.peek(1)?; + storage_target = StorageTarget::Slot(address, index); GasCost::SStore { original: handler.original_storage(address, index), @@ -560,6 +582,7 @@ pub fn dynamic_opcode_cost( }, Opcode::SUICIDE if !is_static => { let target = stack.peek(0)?.into(); + storage_target = StorageTarget::Address(target); GasCost::Suicide { value: handler.balance(address), target_is_cold: handler.is_cold(target, None), @@ -572,6 +595,7 @@ pub fn dynamic_opcode_cost( || (is_static && U256::from_big_endian(&stack.peek(2)?[..]) == U256::zero()) => { let target = stack.peek(1)?.into(); + storage_target = StorageTarget::Address(target); GasCost::Call { value: U256::from_big_endian(&stack.peek(2)?[..]), gas: U256::from_big_endian(&stack.peek(0)?[..]), @@ -646,7 +670,7 @@ pub fn dynamic_opcode_cost( _ => None, }; - Ok((gas_cost, memory_cost)) + Ok((gas_cost, storage_target, memory_cost)) } /// Holds the gas consumption for a Gasometer instance. @@ -948,6 +972,17 @@ pub enum GasCost { }, } +/// Storage opcode will access. Used for tracking accessed storage (EIP-2929). +#[derive(Debug, Clone, Copy)] +pub enum StorageTarget { + /// No storage access + None, + /// Accessing address + Address(H160), + /// Accessing storage slot within an address + Slot(H160, H256), +} + /// Memory cost. #[derive(Debug, Clone, Copy)] pub struct MemoryCost { diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 8a20001c5..eab1cb8e7 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -2,7 +2,7 @@ mod state; pub use self::state::{MemoryStackState, MemoryStackSubstate, StackState}; -use crate::gasometer::{self, Gasometer}; +use crate::gasometer::{self, Gasometer, StorageTarget}; use crate::{ Capture, Config, Context, CreateScheme, ExitError, ExitReason, ExitSucceed, Handler, Opcode, Runtime, Stack, Transfer, @@ -111,7 +111,7 @@ impl<'config> StackSubstateMetadata<'config> { None => Some(0), Some(n) => Some(n + 1), }, - accessed: Default::default(), + accessed: self.accessed.as_ref().map(|_| Accessed::default()), } } @@ -146,6 +146,12 @@ impl<'config> StackSubstateMetadata<'config> { } } + fn access_storage(&mut self, address: H160, key: H256) { + if let Some(accessed) = &mut self.accessed { + accessed.accessed_storage.insert((address, key)); + } + } + fn access_storages(&mut self, storages: I) where I: Iterator, @@ -837,8 +843,8 @@ impl<'config, S: StackState<'config>, P: Precompiles> Handler for StackExecutor< fn is_cold(&self, address: H160, maybe_index: Option) -> bool { match maybe_index { - None => self.state.is_known(address), - Some(index) => self.state.is_storage_known(address, index), + None => self.state.is_cold(address), + Some(index) => self.state.is_storage_cold(address, index), } } @@ -953,7 +959,7 @@ impl<'config, S: StackState<'config>, P: Precompiles> Handler for StackExecutor< self.state.metadata_mut().gasometer.record_cost(cost)?; } else { let is_static = self.state.metadata().is_static; - let (gas_cost, memory_cost) = gasometer::dynamic_opcode_cost( + let (gas_cost, target, memory_cost) = gasometer::dynamic_opcode_cost( context.address, opcode, stack, @@ -965,6 +971,15 @@ impl<'config, S: StackState<'config>, P: Precompiles> Handler for StackExecutor< let gasometer = &mut self.state.metadata_mut().gasometer; gasometer.record_dynamic_cost(gas_cost, memory_cost)?; + match target { + StorageTarget::Address(address) => { + self.state.metadata_mut().access_address(address) + } + StorageTarget::Slot(address, key) => { + self.state.metadata_mut().access_storage(address, key) + } + StorageTarget::None => (), + } } Ok(()) diff --git a/src/executor/stack/state.rs b/src/executor/stack/state.rs index 81ba27bb1..96ea7a95e 100644 --- a/src/executor/stack/state.rs +++ b/src/executor/stack/state.rs @@ -1,5 +1,5 @@ use crate::backend::{Apply, Backend, Basic, Log}; -use crate::executor::stack::StackSubstateMetadata; +use crate::executor::stack::{Accessed, StackSubstateMetadata}; use crate::{ExitError, Transfer}; use alloc::{ boxed::Box, @@ -237,6 +237,26 @@ impl<'config> MemoryStackSubstate<'config> { None } + pub fn is_cold(&self, address: H160) -> bool { + self.recursive_is_cold(&|a| a.accessed_addresses.contains(&address)) + } + + pub fn is_storage_cold(&self, address: H160, key: H256) -> bool { + self.recursive_is_cold(&|a: &Accessed| a.accessed_storage.contains(&(address, key))) + } + + fn recursive_is_cold bool>(&self, f: &F) -> bool { + let local_is_accessed = self.metadata.accessed.as_ref().map(f).unwrap_or(false); + if local_is_accessed { + false + } else { + self.parent + .as_ref() + .map(|p| p.recursive_is_cold(f)) + .unwrap_or(true) + } + } + pub fn deleted(&self, address: H160) -> bool { if self.deletes.contains(&address) { return true; @@ -375,8 +395,8 @@ pub trait StackState<'config>: Backend { fn is_empty(&self, address: H160) -> bool; fn deleted(&self, address: H160) -> bool; - fn is_known(&self, address: H160) -> bool; - fn is_storage_known(&self, address: H160, key: H256) -> bool; + fn is_cold(&self, address: H160) -> bool; + fn is_storage_cold(&self, address: H160, key: H256) -> bool; fn inc_nonce(&mut self, address: H160); fn set_storage(&mut self, address: H160, key: H256, value: H256); @@ -493,12 +513,12 @@ impl<'backend, 'config, B: Backend> StackState<'config> for MemoryStackState<'ba self.substate.deleted(address) } - fn is_known(&self, address: H160) -> bool { - self.substate.known_account(address).is_some() + fn is_cold(&self, address: H160) -> bool { + self.substate.is_cold(address) } - fn is_storage_known(&self, address: H160, key: H256) -> bool { - self.substate.known_storage(address, key).is_some() + fn is_storage_cold(&self, address: H160, key: H256) -> bool { + self.substate.is_storage_cold(address, key) } fn inc_nonce(&mut self, address: H160) { From a252024271a0c2a8ac9661d35f81d4e27e923cce Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Mon, 2 Aug 2021 20:05:03 +0000 Subject: [PATCH 36/38] Move, do not copy, access struct in swallow_commit --- src/executor/stack/mod.rs | 37 +++++++++++-------------------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index eab1cb8e7..ea9802dd7 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -47,14 +47,6 @@ impl Accessed { self.accessed_storage.insert((storage.0, storage.1)); } } - - fn accessed_addresses(&self) -> alloc::collections::btree_set::Iter<'_, H160> { - self.accessed_addresses.iter() - } - - fn accessed_storages(&self) -> alloc::collections::btree_set::Iter<'_, (H160, H256)> { - self.accessed_storage.iter() - } } pub struct StackSubstateMetadata<'config> { @@ -83,12 +75,17 @@ impl<'config> StackSubstateMetadata<'config> { self.gasometer.record_stipend(other.gasometer.gas())?; self.gasometer .record_refund(other.gasometer.refunded_gas())?; - if let Some(addresses) = other.accessed_addresses() { - self.access_addresses(addresses.copied()); - }; - if let Some(storages) = other.accessed_storages() { - self.access_storages(storages.copied()); - }; + + if let (Some(mut other_accessed), Some(self_accessed)) = + (other.accessed, self.accessed.as_mut()) + { + self_accessed + .accessed_addresses + .append(&mut other_accessed.accessed_addresses); + self_accessed + .accessed_storage + .append(&mut other_accessed.accessed_storage); + } Ok(()) } @@ -160,18 +157,6 @@ impl<'config> StackSubstateMetadata<'config> { accessed.access_storages(storages); } } - - fn accessed_addresses(&self) -> Option> { - self.accessed - .as_ref() - .map(|accessed| accessed.accessed_addresses()) - } - - fn accessed_storages(&self) -> Option> { - self.accessed - .as_ref() - .map(|accessed| accessed.accessed_storages()) - } } #[derive(Debug, Eq, PartialEq, Clone)] From 5bad029315c35c77841b7a457c2a59ddcf370db2 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Mon, 9 Aug 2021 17:14:04 +0700 Subject: [PATCH 37/38] Use precompile type over trait --- src/executor/mod.rs | 2 +- src/executor/stack/mod.rs | 93 ++++++++++++--------------------------- 2 files changed, 29 insertions(+), 66 deletions(-) diff --git a/src/executor/mod.rs b/src/executor/mod.rs index 792274c15..08a8c8d67 100644 --- a/src/executor/mod.rs +++ b/src/executor/mod.rs @@ -6,6 +6,6 @@ mod stack; pub use self::stack::{ - MemoryStackState, PrecompileOutput, Precompiles, StackExecutor, StackExitKind, StackState, + MemoryStackState, Precompile, PrecompileOutput, StackExecutor, StackExitKind, StackState, StackSubstateMetadata, }; diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index ea9802dd7..12f275a3f 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -7,7 +7,11 @@ use crate::{ Capture, Config, Context, CreateScheme, ExitError, ExitReason, ExitSucceed, Handler, Opcode, Runtime, Stack, Transfer, }; -use alloc::{collections::BTreeSet, rc::Rc, vec::Vec}; +use alloc::{ + collections::{BTreeMap, BTreeSet}, + rc::Rc, + vec::Vec, +}; use core::{cmp::min, convert::Infallible}; use ethereum::Log; use primitive_types::{H160, H256, U256}; @@ -167,71 +171,35 @@ pub struct PrecompileOutput { pub logs: Vec, } -/// Precompiles trait which allows for the execution of a precompile. -pub trait Precompiles { - /// Runs the precompile at the given address with input and context. - fn run( - &self, - address: H160, - input: &[u8], - gas_limit: Option, - context: &Context, - state: &mut S, - is_static: bool, - ) -> Option>; - - /// Returns the set of precompile addresses. - fn addresses(&self) -> &[H160]; -} - -#[derive(Default)] -pub struct NoPrecompile { - addresses: Vec, -} +/// A precompile result. +pub type PrecompileResult = Result; -impl Precompiles for NoPrecompile { - fn run( - &self, - _address: H160, - _input: &[u8], - _gas_limit: Option, - _context: &Context, - _state: &mut S, - _is_static: bool, - ) -> Option> { - None - } +/// Precompiles function signature. Expected input arguments are: +/// * Input +/// * Context +pub type PrecompileFn = fn(&[u8], Option, &Context) -> PrecompileResult; - fn addresses(&self) -> &[H160] { - &self.addresses - } -} +/// A map of address keys to precompile function values. +pub type Precompile = BTreeMap; /// Stack-based executor. -pub struct StackExecutor<'config, S, P: Precompiles> { +pub struct StackExecutor<'config, S> { config: &'config Config, - precompiles: P, + precompile: Precompile, state: S, } -impl<'config, S: StackState<'config>> StackExecutor<'config, S, NoPrecompile> { - /// Create a new stack-based executor. - pub fn new(state: S, config: &'config Config) -> Self { - Self::new_with_precompile(state, config, NoPrecompile::default()) - } -} - -impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, P> { +impl<'config, S: StackState<'config>> StackExecutor<'config, S> { /// Return a reference of the Config. pub fn config(&self) -> &'config Config { self.config } /// Create a new stack-based executor with given precompiles. - pub fn new_with_precompile(state: S, config: &'config Config, precompile: P) -> Self { + pub fn new_with_precompile(state: S, config: &'config Config, precompile: Precompile) -> Self { Self { config, - precompiles: precompile, + precompile, state, } } @@ -369,10 +337,10 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, // Initialize initial addresses for EIP-2929 if self.config.increase_state_access_gas { let addresses = self - .precompiles - .addresses() - .iter() - .copied() + .precompile + .clone() + .into_keys() + .into_iter() .chain(core::iter::once(caller)) .chain(core::iter::once(address)); self.state.metadata_mut().access_addresses(addresses); @@ -489,9 +457,11 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, self.state.metadata_mut().access_address(caller); self.state.metadata_mut().access_address(address); + + let addresses: Vec = self.precompile.clone().into_keys().collect(); self.state .metadata_mut() - .access_addresses(self.precompiles.addresses().iter().copied()); + .access_addresses(addresses.iter().copied()); event!(Create { caller, @@ -715,15 +685,8 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, } } - if let Some(ret) = self.precompiles.run( - code_address, - &input, - Some(gas_limit), - &context, - &mut self.state, - is_static, - ) { - return match ret { + if let Some(precompile) = self.precompile.get(&code_address) { + return match (*precompile)(&input, Some(gas_limit), &context) { Ok(PrecompileOutput { exit_status, output, @@ -782,7 +745,7 @@ impl<'config, S: StackState<'config>, P: Precompiles> StackExecutor<'config, S, } } -impl<'config, S: StackState<'config>, P: Precompiles> Handler for StackExecutor<'config, S, P> { +impl<'config, S: StackState<'config>> Handler for StackExecutor<'config, S> { type CreateInterrupt = Infallible; type CreateFeedback = Infallible; type CallInterrupt = Infallible; From 865af75411cbfcfed379fd8748421bd37b8c0c59 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Tue, 10 Aug 2021 17:45:15 +0700 Subject: [PATCH 38/38] Fix min version of ethereum dependency --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index b0b071a4f..8491517e0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,7 @@ rlp = { version = "0.5", default-features = false } primitive-types = { version = "0.10", default-features = false, features = ["rlp"] } serde = { version = "1.0", default-features = false, features = ["derive"], optional = true } codec = { package = "parity-scale-codec", version = "2.0", default-features = false, features = ["derive"], optional = true } -ethereum = { version = ">= 0.8, <= 0.9", default-features = false } +ethereum = { version = "~0.8", default-features = false } environmental = { version = "1.1.2", default-features = false, optional = true } [dev-dependencies]