Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Implement EIP-1052 (EXTCODEHASH) and fix several issues in state account cache #9234

Merged
merged 5 commits into from
Jul 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ethcore/evm/src/instructions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ enum_with_from_u8! {
RETURNDATASIZE = 0x3d,
#[doc = "copy return data buffer to memory"]
RETURNDATACOPY = 0x3e,
#[doc = "return the keccak256 hash of contract code"]
EXTCODEHASH = 0x3f,

#[doc = "get hash of most recent complete block"]
BLOCKHASH = 0x40,
Expand Down Expand Up @@ -492,6 +494,7 @@ lazy_static! {
arr[CALLDATALOAD as usize] = Some(InstructionInfo::new("CALLDATALOAD", 1, 1, GasPriceTier::VeryLow));
arr[CALLDATASIZE as usize] = Some(InstructionInfo::new("CALLDATASIZE", 0, 1, GasPriceTier::Base));
arr[CALLDATACOPY as usize] = Some(InstructionInfo::new("CALLDATACOPY", 3, 0, GasPriceTier::VeryLow));
arr[EXTCODEHASH as usize] = Some(InstructionInfo::new("EXTCODEHASH", 1, 1, GasPriceTier::Special));
arr[CODESIZE as usize] = Some(InstructionInfo::new("CODESIZE", 0, 1, GasPriceTier::Base));
arr[CODECOPY as usize] = Some(InstructionInfo::new("CODECOPY", 3, 0, GasPriceTier::VeryLow));
arr[GASPRICE as usize] = Some(InstructionInfo::new("GASPRICE", 0, 1, GasPriceTier::Base));
Expand Down
3 changes: 3 additions & 0 deletions ethcore/evm/src/interpreter/gasometer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ impl<Gas: evm::CostType> Gasometer<Gas> {
instructions::EXTCODESIZE => {
Request::Gas(Gas::from(schedule.extcodesize_gas))
},
instructions::EXTCODEHASH => {
Request::Gas(Gas::from(schedule.extcodehash_gas))
},
instructions::SUICIDE => {
let mut gas = Gas::from(schedule.suicide_gas);

Expand Down
18 changes: 14 additions & 4 deletions ethcore/evm/src/interpreter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,9 @@ impl<Cost: CostType> Interpreter<Cost> {
(instruction == instructions::STATICCALL && !schedule.have_static_call) ||
((instruction == instructions::RETURNDATACOPY || instruction == instructions::RETURNDATASIZE) && !schedule.have_return_data) ||
(instruction == instructions::REVERT && !schedule.have_revert) ||
((instruction == instructions::SHL || instruction == instructions::SHR || instruction == instructions::SAR) && !schedule.have_bitwise_shifting) {

((instruction == instructions::SHL || instruction == instructions::SHR || instruction == instructions::SAR) && !schedule.have_bitwise_shifting) ||
(instruction == instructions::EXTCODEHASH && !schedule.have_extcodehash)
{
return Err(vm::Error::BadInstruction {
instruction: instruction as u8
});
Expand Down Expand Up @@ -568,9 +569,14 @@ impl<Cost: CostType> Interpreter<Cost> {
},
instructions::EXTCODESIZE => {
let address = u256_to_address(&stack.pop_back());
let len = ext.extcodesize(&address)?;
let len = ext.extcodesize(&address)?.unwrap_or(0);
stack.push(U256::from(len));
},
instructions::EXTCODEHASH => {
let address = u256_to_address(&stack.pop_back());
let hash = ext.extcodehash(&address)?.unwrap_or_else(H256::zero);
stack.push(U256::from(hash));
},
instructions::CALLDATACOPY => {
Self::copy_data_to_memory(&mut self.mem, stack, params.data.as_ref().map_or_else(|| &[] as &[u8], |d| &*d as &[u8]));
},
Expand All @@ -591,7 +597,11 @@ impl<Cost: CostType> Interpreter<Cost> {
instructions::EXTCODECOPY => {
let address = u256_to_address(&stack.pop_back());
let code = ext.extcode(&address)?;
Self::copy_data_to_memory(&mut self.mem, stack, &code);
Self::copy_data_to_memory(
&mut self.mem,
stack,
code.as_ref().map(|c| &(*c)[..]).unwrap_or(&[])
);
},
instructions::GASPRICE => {
stack.push(params.gas_price.clone());
Expand Down
2 changes: 1 addition & 1 deletion ethcore/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1344,7 +1344,7 @@ impl BlockInfo for Client {
}

fn code_hash(&self, address: &Address, id: BlockId) -> Option<H256> {
self.state_at(id).and_then(|s| s.code_hash(address).ok())
self.state_at(id).and_then(|s| s.code_hash(address).unwrap_or(None))
}
}

Expand Down
2 changes: 1 addition & 1 deletion ethcore/src/executive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
gas_price: t.gas_price,
value: ActionValue::Transfer(t.value),
code: self.state.code(address)?,
code_hash: Some(self.state.code_hash(address)?),
code_hash: self.state.code_hash(address)?,
data: Some(t.data.clone()),
call_type: CallType::Call,
params_type: vm::ParamsType::Separate,
Expand Down
16 changes: 10 additions & 6 deletions ethcore/src/externalities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B>
gas: self.machine.params().eip210_contract_gas,
gas_price: 0.into(),
code: code,
code_hash: Some(code_hash),
code_hash: code_hash,
data: Some(H256::from(number).to_vec()),
call_type: CallType::Call,
params_type: vm::ParamsType::Separate,
Expand Down Expand Up @@ -272,7 +272,7 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B>
gas: *gas,
gas_price: self.origin_info.gas_price,
code: code,
code_hash: Some(code_hash),
code_hash: code_hash,
data: Some(data.to_vec()),
call_type: call_type,
params_type: vm::ParamsType::Separate,
Expand All @@ -291,12 +291,16 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B>
}
}

fn extcode(&self, address: &Address) -> vm::Result<Arc<Bytes>> {
Ok(self.state.code(address)?.unwrap_or_else(|| Arc::new(vec![])))
fn extcode(&self, address: &Address) -> vm::Result<Option<Arc<Bytes>>> {
Ok(self.state.code(address)?)
}

fn extcodehash(&self, address: &Address) -> vm::Result<Option<H256>> {
Ok(self.state.code_hash(address)?)
}

fn extcodesize(&self, address: &Address) -> vm::Result<usize> {
Ok(self.state.code_size(address)?.unwrap_or(0))
fn extcodesize(&self, address: &Address) -> vm::Result<Option<usize>> {
Ok(self.state.code_size(address)?)
}

fn ret(mut self, gas: &U256, data: &ReturnData, apply_state: bool) -> vm::Result<U256>
Expand Down
8 changes: 6 additions & 2 deletions ethcore/src/json_tests/executive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,18 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for TestExt<'a, T, V, B>
MessageCallResult::Success(*gas, ReturnData::empty())
}

fn extcode(&self, address: &Address) -> vm::Result<Arc<Bytes>> {
fn extcode(&self, address: &Address) -> vm::Result<Option<Arc<Bytes>>> {
self.ext.extcode(address)
}

fn extcodesize(&self, address: &Address) -> vm::Result<usize> {
fn extcodesize(&self, address: &Address) -> vm::Result<Option<usize>> {
self.ext.extcodesize(address)
}

fn extcodehash(&self, address: &Address) -> vm::Result<Option<H256>> {
self.ext.extcodehash(address)
}

fn log(&mut self, topics: Vec<H256>, data: &[u8]) -> vm::Result<()> {
self.ext.log(topics, data)
}
Expand Down
2 changes: 1 addition & 1 deletion ethcore/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl EthereumMachine {
gas_price: 0.into(),
value: ActionValue::Transfer(0.into()),
code: state.code(&contract_address)?,
code_hash: Some(state.code_hash(&contract_address)?),
code_hash: state.code_hash(&contract_address)?,
data: data,
call_type: CallType::Call,
params_type: ParamsType::Separate,
Expand Down
7 changes: 7 additions & 0 deletions ethcore/src/spec/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ pub struct CommonParams {
pub eip214_transition: BlockNumber,
/// Number of first block where EIP-145 rules begin.
pub eip145_transition: BlockNumber,
/// Number of first block where EIP-1052 rules begin.
pub eip1052_transition: BlockNumber,
/// Number of first block where dust cleanup rules (EIP-168 and EIP169) begin.
pub dust_protection_transition: BlockNumber,
/// Nonce cap increase per block. Nonce cap is only checked if dust protection is enabled.
Expand Down Expand Up @@ -174,6 +176,7 @@ impl CommonParams {
schedule.have_static_call = block_number >= self.eip214_transition;
schedule.have_return_data = block_number >= self.eip211_transition;
schedule.have_bitwise_shifting = block_number >= self.eip145_transition;
schedule.have_extcodehash = block_number >= self.eip1052_transition;
if block_number >= self.eip210_transition {
schedule.blockhash_gas = 800;
}
Expand Down Expand Up @@ -270,6 +273,10 @@ impl From<ethjson::spec::Params> for CommonParams {
BlockNumber::max_value,
Into::into,
),
eip1052_transition: p.eip1052_transition.map_or_else(
BlockNumber::max_value,
Into::into,
),
dust_protection_transition: p.dust_protection_transition.map_or_else(
BlockNumber::max_value,
Into::into,
Expand Down
16 changes: 10 additions & 6 deletions ethcore/src/state/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,13 @@ impl Account {
!self.code_cache.is_empty() || (self.code_cache.is_empty() && self.code_hash == KECCAK_EMPTY)
}

/// Provide a database to get `code_hash`. Should not be called if it is a contract without code.
/// Provide a database to get `code_hash`. Should not be called if it is a contract without code. Returns the cached code, if successful.
#[must_use]
pub fn cache_code(&mut self, db: &HashDB<KeccakHasher>) -> Option<Arc<Bytes>> {
// TODO: fill out self.code_cache;
trace!("Account::cache_code: ic={}; self.code_hash={:?}, self.code_cache={}", self.is_cached(), self.code_hash, self.code_cache.pretty());

if self.is_cached() { return Some(self.code_cache.clone()) }
if self.is_cached() { return Some(self.code_cache.clone()); }

match db.get(&self.code_hash) {
Some(x) => {
Expand All @@ -298,16 +299,17 @@ impl Account {
}
}

/// Provide code to cache. For correctness, should be the correct code for the
/// account.
/// Provide code to cache. For correctness, should be the correct code for the account.
pub fn cache_given_code(&mut self, code: Arc<Bytes>) {
trace!("Account::cache_given_code: ic={}; self.code_hash={:?}, self.code_cache={}", self.is_cached(), self.code_hash, self.code_cache.pretty());

self.code_size = Some(code.len());
self.code_cache = code;
}

/// Provide a database to get `code_size`. Should not be called if it is a contract without code.
/// Provide a database to get `code_size`. Should not be called if it is a contract without code. Returns whether
/// the cache succeeds.
#[must_use]
pub fn cache_code_size(&mut self, db: &HashDB<KeccakHasher>) -> bool {
// TODO: fill out self.code_cache;
trace!("Account::cache_code_size: ic={}; self.code_hash={:?}, self.code_cache={}", self.is_cached(), self.code_hash, self.code_cache.pretty());
Expand All @@ -324,7 +326,9 @@ impl Account {
},
}
} else {
false
// If the code hash is empty hash, then the code size is zero.
self.code_size = Some(0);
true
}
}

Expand Down
46 changes: 30 additions & 16 deletions ethcore/src/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,9 +608,9 @@ impl<B: Backend> State<B> {
}

/// Get an account's code hash.
pub fn code_hash(&self, a: &Address) -> TrieResult<H256> {
pub fn code_hash(&self, a: &Address) -> TrieResult<Option<H256>> {
self.ensure_cached(a, RequireCache::None, true,
|a| a.as_ref().map_or(KECCAK_EMPTY, |a| a.code_hash()))
|a| a.as_ref().map(|a| a.code_hash()))
}

/// Get accounts' code size.
Expand Down Expand Up @@ -911,31 +911,38 @@ impl<B: Backend> State<B> {
Ok(pod_state::diff_pod(&pod_state_pre, &pod_state_post))
}

// load required account data from the databases.
fn update_account_cache(require: RequireCache, account: &mut Account, state_db: &B, db: &HashDB<KeccakHasher>) {
/// Load required account data from the databases. Returns whether the cache succeeds.
#[must_use]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 We should probably use this more.

fn update_account_cache(require: RequireCache, account: &mut Account, state_db: &B, db: &HashDB<KeccakHasher>) -> bool {
if let RequireCache::None = require {
return;
return true;
}

if account.is_cached() {
return;
return true;
}

// if there's already code in the global cache, always cache it localy
let hash = account.code_hash();
match state_db.get_cached_code(&hash) {
Some(code) => account.cache_given_code(code),
Some(code) => {
account.cache_given_code(code);
true
},
None => match require {
RequireCache::None => {},
RequireCache::None => true,
RequireCache::Code => {
if let Some(code) = account.cache_code(db) {
// propagate code loaded from the database to
// the global code cache.
state_db.cache_code(hash, code)
state_db.cache_code(hash, code);
true
} else {
false
}
},
RequireCache::CodeSize => {
account.cache_code_size(db);
account.cache_code_size(db)
}
}
}
Expand All @@ -950,21 +957,26 @@ impl<B: Backend> State<B> {
if let Some(ref mut maybe_acc) = self.cache.borrow_mut().get_mut(a) {
if let Some(ref mut account) = maybe_acc.account {
let accountdb = self.factories.accountdb.readonly(self.db.as_hashdb(), account.address_hash(a));
Self::update_account_cache(require, account, &self.db, accountdb.as_hashdb());
return Ok(f(Some(account)));
if Self::update_account_cache(require, account, &self.db, accountdb.as_hashdb()) {
return Ok(f(Some(account)));
} else {
return Err(Box::new(TrieError::IncompleteDatabase(H256::from(a))));
}
}
return Ok(f(None));
}
// check global cache
let result = self.db.get_cached(a, |mut acc| {
if let Some(ref mut account) = acc {
let accountdb = self.factories.accountdb.readonly(self.db.as_hashdb(), account.address_hash(a));
Self::update_account_cache(require, account, &self.db, accountdb.as_hashdb());
if !Self::update_account_cache(require, account, &self.db, accountdb.as_hashdb()) {
return Err(Box::new(TrieError::IncompleteDatabase(H256::from(a))));
}
}
f(acc.map(|a| &*a))
Ok(f(acc.map(|a| &*a)))
});
match result {
Some(r) => Ok(r),
Some(r) => Ok(r?),
None => {
// first check if it is not in database for sure
if check_null && self.db.is_known_null(a) { return Ok(f(None)); }
Expand All @@ -975,7 +987,9 @@ impl<B: Backend> State<B> {
let mut maybe_acc = db.get_with(a, from_rlp)?;
if let Some(ref mut account) = maybe_acc.as_mut() {
let accountdb = self.factories.accountdb.readonly(self.db.as_hashdb(), account.address_hash(a));
Self::update_account_cache(require, account, &self.db, accountdb.as_hashdb());
if !Self::update_account_cache(require, account, &self.db, accountdb.as_hashdb()) {
return Err(Box::new(TrieError::IncompleteDatabase(H256::from(a))));
}
}
let r = f(maybe_acc.as_ref());
self.insert_cache(a, AccountEntry::new_clean(maybe_acc));
Expand Down
7 changes: 5 additions & 2 deletions ethcore/vm/src/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,13 @@ pub trait Ext {
) -> MessageCallResult;

/// Returns code at given address
fn extcode(&self, address: &Address) -> Result<Arc<Bytes>>;
fn extcode(&self, address: &Address) -> Result<Option<Arc<Bytes>>>;

/// Returns code hash at given address
fn extcodehash(&self, address: &Address) -> Result<Option<H256>>;

/// Returns code size at given address
fn extcodesize(&self, address: &Address) -> Result<usize>;
fn extcodesize(&self, address: &Address) -> Result<Option<usize>>;

/// Creates log entry with given topics and data
fn log(&mut self, topics: Vec<H256>, data: &[u8]) -> Result<()>;
Expand Down
8 changes: 8 additions & 0 deletions ethcore/vm/src/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ pub struct Schedule {
pub have_create2: bool,
/// Does it have a REVERT instruction
pub have_revert: bool,
/// Does it have a EXTCODEHASH instruction
pub have_extcodehash: bool,
/// VM stack limit
pub stack_limit: usize,
/// Max number of nested calls/creates
Expand Down Expand Up @@ -92,6 +94,8 @@ pub struct Schedule {
pub extcodecopy_base_gas: usize,
/// Price of BALANCE
pub balance_gas: usize,
/// Price of EXTCODEHASH
pub extcodehash_gas: usize,
/// Price of SUICIDE
pub suicide_gas: usize,
/// Amount of additional gas to pay when SUICIDE credits a non-existant account
Expand Down Expand Up @@ -197,6 +201,7 @@ impl Schedule {
have_revert: false,
have_return_data: false,
have_bitwise_shifting: false,
have_extcodehash: false,
stack_limit: 1024,
max_depth: 1024,
tier_step_gas: [0, 2, 3, 5, 8, 10, 20, 0],
Expand Down Expand Up @@ -229,6 +234,7 @@ impl Schedule {
copy_gas: 3,
extcodesize_gas: 700,
extcodecopy_base_gas: 700,
extcodehash_gas: 400,
balance_gas: 400,
suicide_gas: 5000,
suicide_to_new_account_cost: 25000,
Expand Down Expand Up @@ -268,6 +274,7 @@ impl Schedule {
have_revert: false,
have_return_data: false,
have_bitwise_shifting: false,
have_extcodehash: false,
stack_limit: 1024,
max_depth: 1024,
tier_step_gas: [0, 2, 3, 5, 8, 10, 20, 0],
Expand Down Expand Up @@ -300,6 +307,7 @@ impl Schedule {
copy_gas: 3,
extcodesize_gas: 20,
extcodecopy_base_gas: 20,
extcodehash_gas: 400,
balance_gas: 20,
suicide_gas: 0,
suicide_to_new_account_cost: 0,
Expand Down
Loading