From 915c36605638876629f93725887ce5d655bc0376 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Fri, 7 Sep 2018 18:51:08 +0800 Subject: [PATCH] EIP 1283: Net gas metering for SSTORE without dirty maps (#9319) * Implement last_checkpoint_storage_at * Add reverted_storage_at for externalities * sstore_clears_count -> sstore_clears_refund * Implement eip1283 for evm * Add eip1283Transition params * evm: fix tests * jsontests: fix test * Return checkpoint index when creating * Comply with spec Version II * Fix docs * Fix jsontests feature compile * Address grumbles * Fix no-checkpoint-entry case * Remove unnecessary expect * Add test for State::checkpoint_storage_at * Add executive level test for eip1283 * Hard-code transaction_checkpoint_index to 0 * Fix jsontests * Add tests for checkpoint discard/revert * Require checkpoint to be empty for kill_account and commit * Get code coverage * Use saturating_add/saturating_sub * Fix issues in insert_cache * Clear the state again * Fix original_storage_at * Early return for empty RLP trie storage * Update comments * Fix borrow_mut issue * Simplify checkpoint_storage_at if branches * Better commenting for gas handling code * Address naming grumbles * More tests * Fix an issue in overwrite_with * Add another test * Fix comment * Remove unnecessary bracket * Move orig to inner if * Remove test coverage for this PR * Add tests for executive original value * Add warn! for an unreachable cause --- ethcore/evm/src/interpreter/gasometer.rs | 98 +++++- ethcore/evm/src/interpreter/mod.rs | 10 +- ethcore/evm/src/tests.rs | 2 +- ethcore/res/ethereum/constantinople_test.json | 3 +- ethcore/src/executive.rs | 66 +++- ethcore/src/externalities.rs | 12 +- ethcore/src/json_tests/executive.rs | 12 +- ethcore/src/spec/spec.rs | 7 + ethcore/src/state/account.rs | 143 ++++++-- ethcore/src/state/mod.rs | 304 ++++++++++++++++-- ethcore/src/state/substate.rs | 12 +- ethcore/vm/src/ext.rs | 10 +- ethcore/vm/src/schedule.rs | 4 + ethcore/vm/src/tests.rs | 14 +- ethcore/wasm/src/runtime.rs | 3 +- json/src/spec/params.rs | 2 + 16 files changed, 634 insertions(+), 68 deletions(-) diff --git a/ethcore/evm/src/interpreter/gasometer.rs b/ethcore/evm/src/interpreter/gasometer.rs index 78a33ef3e..406df19fd 100644 --- a/ethcore/evm/src/interpreter/gasometer.rs +++ b/ethcore/evm/src/interpreter/gasometer.rs @@ -125,12 +125,17 @@ impl Gasometer { let newval = stack.peek(1); let val = U256::from(&*ext.storage_at(&address)?); - let gas = if val.is_zero() && !newval.is_zero() { - schedule.sstore_set_gas + let gas = if schedule.eip1283 { + let orig = U256::from(&*ext.initial_storage_at(&address)?); + calculate_eip1283_sstore_gas(schedule, &orig, &val, &newval) } else { - // Refund for below case is added when actually executing sstore - // !is_zero(&val) && is_zero(newval) - schedule.sstore_reset_gas + if val.is_zero() && !newval.is_zero() { + schedule.sstore_set_gas + } else { + // Refund for below case is added when actually executing sstore + // !is_zero(&val) && is_zero(newval) + schedule.sstore_reset_gas + } }; Request::Gas(Gas::from(gas)) }, @@ -342,6 +347,89 @@ fn add_gas_usize(value: Gas, num: usize) -> (Gas, bool) { value.overflow_add(Gas::from(num)) } +#[inline] +fn calculate_eip1283_sstore_gas(schedule: &Schedule, original: &U256, current: &U256, new: &U256) -> Gas { + Gas::from( + if current == new { + // 1. If current value equals new value (this is a no-op), 200 gas is deducted. + schedule.sload_gas + } else { + // 2. If current value does not equal new value + if original == current { + // 2.1. If original value equals current value (this storage slot has not been changed by the current execution context) + if original.is_zero() { + // 2.1.1. If original value is 0, 20000 gas is deducted. + schedule.sstore_set_gas + } else { + // 2.1.2. Otherwise, 5000 gas is deducted. + schedule.sstore_reset_gas + + // 2.1.2.1. If new value is 0, add 15000 gas to refund counter. + } + } else { + // 2.2. If original value does not equal current value (this storage slot is dirty), 200 gas is deducted. Apply both of the following clauses. + schedule.sload_gas + + // 2.2.1. If original value is not 0 + // 2.2.1.1. If current value is 0 (also means that new value is not 0), remove 15000 gas from refund counter. We can prove that refund counter will never go below 0. + // 2.2.1.2. If new value is 0 (also means that current value is not 0), add 15000 gas to refund counter. + + // 2.2.2. If original value equals new value (this storage slot is reset) + // 2.2.2.1. If original value is 0, add 19800 gas to refund counter. + // 2.2.2.2. Otherwise, add 4800 gas to refund counter. + } + } + ) +} + +pub fn handle_eip1283_sstore_clears_refund(ext: &mut vm::Ext, original: &U256, current: &U256, new: &U256) { + let sstore_clears_schedule = U256::from(ext.schedule().sstore_refund_gas); + + if current == new { + // 1. If current value equals new value (this is a no-op), 200 gas is deducted. + } else { + // 2. If current value does not equal new value + if original == current { + // 2.1. If original value equals current value (this storage slot has not been changed by the current execution context) + if original.is_zero() { + // 2.1.1. If original value is 0, 20000 gas is deducted. + } else { + // 2.1.2. Otherwise, 5000 gas is deducted. + if new.is_zero() { + // 2.1.2.1. If new value is 0, add 15000 gas to refund counter. + ext.add_sstore_refund(sstore_clears_schedule); + } + } + } else { + // 2.2. If original value does not equal current value (this storage slot is dirty), 200 gas is deducted. Apply both of the following clauses. + + if !original.is_zero() { + // 2.2.1. If original value is not 0 + if current.is_zero() { + // 2.2.1.1. If current value is 0 (also means that new value is not 0), remove 15000 gas from refund counter. We can prove that refund counter will never go below 0. + ext.sub_sstore_refund(sstore_clears_schedule); + } else if new.is_zero() { + // 2.2.1.2. If new value is 0 (also means that current value is not 0), add 15000 gas to refund counter. + ext.add_sstore_refund(sstore_clears_schedule); + } + } + + if original == new { + // 2.2.2. If original value equals new value (this storage slot is reset) + if original.is_zero() { + // 2.2.2.1. If original value is 0, add 19800 gas to refund counter. + let refund = U256::from(ext.schedule().sstore_set_gas - ext.schedule().sload_gas); + ext.add_sstore_refund(refund); + } else { + // 2.2.2.2. Otherwise, add 4800 gas to refund counter. + let refund = U256::from(ext.schedule().sstore_reset_gas - ext.schedule().sload_gas); + ext.add_sstore_refund(refund); + } + } + } + } +} + #[test] fn test_mem_gas_cost() { // given diff --git a/ethcore/evm/src/interpreter/mod.rs b/ethcore/evm/src/interpreter/mod.rs index 5c4f5e680..07d0f0c62 100644 --- a/ethcore/evm/src/interpreter/mod.rs +++ b/ethcore/evm/src/interpreter/mod.rs @@ -629,8 +629,14 @@ impl Interpreter { let current_val = U256::from(&*ext.storage_at(&address)?); // Increase refund for clear - if !current_val.is_zero() && val.is_zero() { - ext.inc_sstore_clears(); + if ext.schedule().eip1283 { + let original_val = U256::from(&*ext.initial_storage_at(&address)?); + gasometer::handle_eip1283_sstore_clears_refund(ext, &original_val, ¤t_val, &val); + } else { + if !current_val.is_zero() && val.is_zero() { + let sstore_clears_schedule = U256::from(ext.schedule().sstore_refund_gas); + ext.add_sstore_refund(sstore_clears_schedule); + } } ext.set_storage(address, H256::from(&val))?; }, diff --git a/ethcore/evm/src/tests.rs b/ethcore/evm/src/tests.rs index b1f6268bf..9fd7bcd90 100644 --- a/ethcore/evm/src/tests.rs +++ b/ethcore/evm/src/tests.rs @@ -716,7 +716,7 @@ fn test_jumps(factory: super::Factory) { test_finalize(vm.exec(&mut ext)).unwrap() }; - assert_eq!(ext.sstore_clears, 1); + assert_eq!(ext.sstore_clears, U256::from(ext.schedule().sstore_refund_gas)); assert_store(&ext, 0, "0000000000000000000000000000000000000000000000000000000000000000"); // 5! assert_store(&ext, 1, "0000000000000000000000000000000000000000000000000000000000000078"); // 5! assert_eq!(gas_left, U256::from(54_117)); diff --git a/ethcore/res/ethereum/constantinople_test.json b/ethcore/res/ethereum/constantinople_test.json index 155b06507..9e7df1f7f 100644 --- a/ethcore/res/ethereum/constantinople_test.json +++ b/ethcore/res/ethereum/constantinople_test.json @@ -33,7 +33,8 @@ "eip211Transition": "0x0", "eip214Transition": "0x0", "eip155Transition": "0x0", - "eip658Transition": "0x0" + "eip658Transition": "0x0", + "eip1283Transition": "0x0" }, "genesis": { "seal": { diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index ff5c54eff..d2cf2f027 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -574,9 +574,9 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { let prev_bal = self.state.balance(¶ms.address)?; if let ActionValue::Transfer(val) = params.value { self.state.sub_balance(¶ms.sender, &val, &mut substate.to_cleanup_mode(&schedule))?; - self.state.new_contract(¶ms.address, val + prev_bal, nonce_offset); + self.state.new_contract(¶ms.address, val + prev_bal, nonce_offset)?; } else { - self.state.new_contract(¶ms.address, prev_bal, nonce_offset); + self.state.new_contract(¶ms.address, prev_bal, nonce_offset)?; } let trace_info = tracer.prepare_trace_create(¶ms); @@ -627,7 +627,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { let schedule = self.schedule; // refunds from SSTORE nonzero -> zero - let sstore_refunds = U256::from(schedule.sstore_refund_gas) * substate.sstore_clears_count; + let sstore_refunds = substate.sstore_clears_refund; // refunds from contract suicides let suicide_refunds = U256::from(schedule.suicide_refund_gas) * U256::from(substate.suicides.len()); let refunds_bound = sstore_refunds + suicide_refunds; @@ -1614,6 +1614,66 @@ mod tests { assert_eq!(state.storage_at(&contract_address, &H256::from(&U256::zero())).unwrap(), H256::from(&U256::from(0))); } + evm_test!{test_eip1283: test_eip1283_int} + fn test_eip1283(factory: Factory) { + let x1 = Address::from(0x1000); + let x2 = Address::from(0x1001); + let y1 = Address::from(0x2001); + let y2 = Address::from(0x2002); + let operating_address = Address::from(0); + let k = H256::new(); + + let mut state = get_temp_state_with_factory(factory.clone()); + state.new_contract(&x1, U256::zero(), U256::from(1)).unwrap(); + state.init_code(&x1, "600160005560006000556001600055".from_hex().unwrap()).unwrap(); + state.new_contract(&x2, U256::zero(), U256::from(1)).unwrap(); + state.init_code(&x2, "600060005560016000556000600055".from_hex().unwrap()).unwrap(); + state.new_contract(&y1, U256::zero(), U256::from(1)).unwrap(); + state.init_code(&y1, "600060006000600061100062fffffff4".from_hex().unwrap()).unwrap(); + state.new_contract(&y2, U256::zero(), U256::from(1)).unwrap(); + state.init_code(&y2, "600060006000600061100162fffffff4".from_hex().unwrap()).unwrap(); + + let info = EnvInfo::default(); + let machine = ::ethereum::new_constantinople_test_machine(); + let schedule = machine.schedule(info.number); + + assert_eq!(state.storage_at(&operating_address, &k).unwrap(), H256::from(U256::from(0))); + // Test a call via top-level -> y1 -> x1 + let (FinalizationResult { gas_left, .. }, refund, gas) = { + let gas = U256::from(0xffffffffffu64); + let mut params = ActionParams::default(); + params.code = Some(Arc::new("6001600055600060006000600061200163fffffffff4".from_hex().unwrap())); + params.gas = gas; + let mut substate = Substate::new(); + let mut ex = Executive::new(&mut state, &info, &machine, &schedule); + let res = ex.call(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer).unwrap(); + + (res, substate.sstore_clears_refund, gas) + }; + let gas_used = gas - gas_left; + // sstore: 0 -> (1) -> () -> (1 -> 0 -> 1) + assert_eq!(gas_used, U256::from(41860)); + assert_eq!(refund, U256::from(19800)); + + assert_eq!(state.storage_at(&operating_address, &k).unwrap(), H256::from(U256::from(1))); + // Test a call via top-level -> y2 -> x2 + let (FinalizationResult { gas_left, .. }, refund, gas) = { + let gas = U256::from(0xffffffffffu64); + let mut params = ActionParams::default(); + params.code = Some(Arc::new("6001600055600060006000600061200263fffffffff4".from_hex().unwrap())); + params.gas = gas; + let mut substate = Substate::new(); + let mut ex = Executive::new(&mut state, &info, &machine, &schedule); + let res = ex.call(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer).unwrap(); + + (res, substate.sstore_clears_refund, gas) + }; + let gas_used = gas - gas_left; + // sstore: 1 -> (1) -> () -> (0 -> 1 -> 0) + assert_eq!(gas_used, U256::from(11860)); + assert_eq!(refund, U256::from(19800)); + } + fn wasm_sample_code() -> Arc> { Arc::new( "0061736d01000000010d0360027f7f0060017f0060000002270303656e7603726574000003656e760673656e646572000103656e76066d656d6f727902010110030201020404017000000501000708010463616c6c00020901000ac10101be0102057f017e4100410028020441c0006b22043602042004412c6a41106a220041003602002004412c6a41086a22014200370200200441186a41106a22024100360200200441186a41086a220342003703002004420037022c2004410036021c20044100360218200441186a1001200020022802002202360200200120032903002205370200200441106a2002360200200441086a200537030020042004290318220537022c200420053703002004411410004100200441c0006a3602040b0b0a010041040b0410c00000" diff --git a/ethcore/src/externalities.rs b/ethcore/src/externalities.rs index dad3987b9..5c7c95c66 100644 --- a/ethcore/src/externalities.rs +++ b/ethcore/src/externalities.rs @@ -113,6 +113,10 @@ impl<'a, T: 'a, V: 'a, B: 'a> Externalities<'a, T, V, B> impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> where T: Tracer, V: VMTracer, B: StateBackend { + fn initial_storage_at(&self, key: &H256) -> vm::Result { + self.state.checkpoint_storage_at(0, &self.origin_info.address, key).map(|v| v.unwrap_or(H256::zero())).map_err(Into::into) + } + fn storage_at(&self, key: &H256) -> vm::Result { self.state.storage_at(&self.origin_info.address, key).map_err(Into::into) } @@ -390,8 +394,12 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> self.depth } - fn inc_sstore_clears(&mut self) { - self.substate.sstore_clears_count = self.substate.sstore_clears_count + U256::one(); + fn add_sstore_refund(&mut self, value: U256) { + self.substate.sstore_clears_refund = self.substate.sstore_clears_refund.saturating_add(value); + } + + fn sub_sstore_refund(&mut self, value: U256) { + self.substate.sstore_clears_refund = self.substate.sstore_clears_refund.saturating_sub(value); } fn trace_next_instruction(&mut self, pc: usize, instruction: u8, current_gas: U256) -> bool { diff --git a/ethcore/src/json_tests/executive.rs b/ethcore/src/json_tests/executive.rs index 20e7ebb20..78ef3e22d 100644 --- a/ethcore/src/json_tests/executive.rs +++ b/ethcore/src/json_tests/executive.rs @@ -112,6 +112,10 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for TestExt<'a, T, V, B> self.ext.storage_at(key) } + fn initial_storage_at(&self, key: &H256) -> vm::Result { + self.ext.initial_storage_at(key) + } + fn set_storage(&mut self, key: H256, value: H256) -> vm::Result<()> { self.ext.set_storage(key, value) } @@ -205,8 +209,12 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for TestExt<'a, T, V, B> false } - fn inc_sstore_clears(&mut self) { - self.ext.inc_sstore_clears() + fn add_sstore_refund(&mut self, value: U256) { + self.ext.add_sstore_refund(value) + } + + fn sub_sstore_refund(&mut self, value: U256) { + self.ext.sub_sstore_refund(value) } } diff --git a/ethcore/src/spec/spec.rs b/ethcore/src/spec/spec.rs index 9ba3d5d30..aa5d9cd9b 100644 --- a/ethcore/src/spec/spec.rs +++ b/ethcore/src/spec/spec.rs @@ -115,6 +115,8 @@ pub struct CommonParams { pub eip145_transition: BlockNumber, /// Number of first block where EIP-1052 rules begin. pub eip1052_transition: BlockNumber, + /// Number of first block where EIP-1283 rules begin. + pub eip1283_transition: BlockNumber, /// Number of first block where EIP-1014 rules begin. pub eip1014_transition: BlockNumber, /// Number of first block where dust cleanup rules (EIP-168 and EIP169) begin. @@ -183,6 +185,7 @@ impl CommonParams { 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; + schedule.eip1283 = block_number >= self.eip1283_transition; if block_number >= self.eip210_transition { schedule.blockhash_gas = 800; } @@ -286,6 +289,10 @@ impl From for CommonParams { BlockNumber::max_value, Into::into, ), + eip1283_transition: p.eip1283_transition.map_or_else( + BlockNumber::max_value, + Into::into, + ), eip1014_transition: p.eip1014_transition.map_or_else( BlockNumber::max_value, Into::into, diff --git a/ethcore/src/state/account.rs b/ethcore/src/state/account.rs index e08e23658..a03312ca4 100644 --- a/ethcore/src/state/account.rs +++ b/ethcore/src/state/account.rs @@ -59,6 +59,10 @@ pub struct Account { // LRU Cache of the trie-backed storage. // This is limited to `STORAGE_CACHE_ITEMS` recent queries storage_cache: RefCell>, + // LRU Cache of the trie-backed storage for original value. + // This is only used when the initial storage root is different compared to + // what is in the database. That is, it is only used for new contracts. + original_storage_cache: Option<(H256, RefCell>)>, // Modified storage. Accumulates changes to storage made in `set_storage` // Takes precedence over `storage_cache`. storage_changes: HashMap, @@ -81,6 +85,7 @@ impl From for Account { nonce: basic.nonce, storage_root: basic.storage_root, storage_cache: Self::empty_storage_cache(), + original_storage_cache: None, storage_changes: HashMap::new(), code_hash: basic.code_hash, code_size: None, @@ -100,6 +105,7 @@ impl Account { nonce: nonce, storage_root: KECCAK_NULL_RLP, storage_cache: Self::empty_storage_cache(), + original_storage_cache: None, storage_changes: storage, code_hash: keccak(&code), code_size: Some(code.len()), @@ -120,6 +126,7 @@ impl Account { nonce: pod.nonce, storage_root: KECCAK_NULL_RLP, storage_cache: Self::empty_storage_cache(), + original_storage_cache: None, storage_changes: pod.storage.into_iter().collect(), code_hash: pod.code.as_ref().map_or(KECCAK_EMPTY, |c| keccak(c)), code_filth: Filth::Dirty, @@ -136,6 +143,7 @@ impl Account { nonce: nonce, storage_root: KECCAK_NULL_RLP, storage_cache: Self::empty_storage_cache(), + original_storage_cache: None, storage_changes: HashMap::new(), code_hash: KECCAK_EMPTY, code_cache: Arc::new(vec![]), @@ -154,12 +162,17 @@ impl Account { /// Create a new contract account. /// NOTE: make sure you use `init_code` on this before `commit`ing. - pub fn new_contract(balance: U256, nonce: U256) -> Account { + pub fn new_contract(balance: U256, nonce: U256, original_storage_root: H256) -> Account { Account { balance: balance, nonce: nonce, storage_root: KECCAK_NULL_RLP, storage_cache: Self::empty_storage_cache(), + original_storage_cache: if original_storage_root == KECCAK_NULL_RLP { + None + } else { + Some((original_storage_root, Self::empty_storage_cache())) + }, storage_changes: HashMap::new(), code_hash: KECCAK_EMPTY, code_cache: Arc::new(vec![]), @@ -204,11 +217,43 @@ impl Account { if let Some(value) = self.cached_storage_at(key) { return Ok(value); } - let db = SecTrieDB::new(db, &self.storage_root)?; + Self::get_and_cache_storage( + &self.storage_root, + &mut self.storage_cache.borrow_mut(), + db, + key) + } + + /// Get (and cache) the contents of the trie's storage at `key`. + /// Does not take modified storage into account. + pub fn original_storage_at(&self, db: &HashDB, key: &H256) -> TrieResult { + if let Some(value) = self.cached_original_storage_at(key) { + return Ok(value); + } + match &self.original_storage_cache { + Some((ref original_storage_root, ref original_storage_cache)) => + Self::get_and_cache_storage( + original_storage_root, + &mut original_storage_cache.borrow_mut(), + db, + key + ), + None => + Self::get_and_cache_storage( + &self.storage_root, + &mut self.storage_cache.borrow_mut(), + db, + key + ), + } + } + + fn get_and_cache_storage(storage_root: &H256, storage_cache: &mut LruCache, db: &HashDB, key: &H256) -> TrieResult { + let db = SecTrieDB::new(db, storage_root)?; let panicky_decoder = |bytes:&[u8]| ::rlp::decode(&bytes).expect("decoding db value failed"); let item: U256 = db.get_with(key, panicky_decoder)?.unwrap_or_else(U256::zero); let value: H256 = item.into(); - self.storage_cache.borrow_mut().insert(key.clone(), value.clone()); + storage_cache.insert(key.clone(), value.clone()); Ok(value) } @@ -218,10 +263,38 @@ impl Account { if let Some(value) = self.storage_changes.get(key) { return Some(value.clone()) } - if let Some(value) = self.storage_cache.borrow_mut().get_mut(key) { - return Some(value.clone()) + self.cached_moved_original_storage_at(key) + } + + /// Get cached original storage value after last state commitment. Returns `None` if the key is not in the cache. + pub fn cached_original_storage_at(&self, key: &H256) -> Option { + match &self.original_storage_cache { + Some((_, ref original_storage_cache)) => { + if let Some(value) = original_storage_cache.borrow_mut().get_mut(key) { + Some(value.clone()) + } else { + None + } + }, + None => { + self.cached_moved_original_storage_at(key) + }, + } + } + + /// Get cached original storage value since last contract creation on this address. Returns `None` if the key is not in the cache. + fn cached_moved_original_storage_at(&self, key: &H256) -> Option { + // If storage root is empty RLP, then early return zero value. Practically, this makes it so that if + // `original_storage_cache` is used, then `storage_cache` will always remain empty. + if self.storage_root == KECCAK_NULL_RLP { + return Some(H256::new()); + } + + if let Some(value) = self.storage_cache.borrow_mut().get_mut(key) { + Some(value.clone()) + } else { + None } - None } /// return the balance associated with this account. @@ -357,7 +430,27 @@ impl Account { } /// Return the storage root associated with this account or None if it has been altered via the overlay. - pub fn storage_root(&self) -> Option<&H256> { if self.storage_is_clean() {Some(&self.storage_root)} else {None} } + pub fn storage_root(&self) -> Option { + if self.storage_is_clean() { + Some(self.storage_root) + } else { + None + } + } + + /// Return the original storage root of this account. + pub fn original_storage_root(&self) -> H256 { + if let Some((original_storage_root, _)) = self.original_storage_cache { + original_storage_root + } else { + self.storage_root + } + } + + /// Storage root where the account changes are based upon. + pub fn base_storage_root(&self) -> H256 { + self.storage_root + } /// Return the storage overlay. pub fn storage_changes(&self) -> &HashMap { &self.storage_changes } @@ -392,6 +485,7 @@ impl Account { self.storage_cache.borrow_mut().insert(k, v); } + self.original_storage_cache = None; Ok(()) } @@ -429,6 +523,7 @@ impl Account { nonce: self.nonce.clone(), storage_root: self.storage_root.clone(), storage_cache: Self::empty_storage_cache(), + original_storage_cache: self.original_storage_cache.as_ref().map(|(r, _)| (*r, Self::empty_storage_cache())), storage_changes: HashMap::new(), code_hash: self.code_hash.clone(), code_size: self.code_size.clone(), @@ -449,6 +544,7 @@ impl Account { pub fn clone_all(&self) -> Account { let mut account = self.clone_dirty(); account.storage_cache = self.storage_cache.clone(); + account.original_storage_cache = self.original_storage_cache.clone(); account } @@ -458,16 +554,21 @@ impl Account { pub fn overwrite_with(&mut self, other: Account) { self.balance = other.balance; self.nonce = other.nonce; - self.storage_root = other.storage_root; self.code_hash = other.code_hash; self.code_filth = other.code_filth; self.code_cache = other.code_cache; self.code_size = other.code_size; self.address_hash = other.address_hash; - let mut cache = self.storage_cache.borrow_mut(); - for (k, v) in other.storage_cache.into_inner() { - cache.insert(k, v); + if self.storage_root == other.storage_root { + let mut cache = self.storage_cache.borrow_mut(); + for (k, v) in other.storage_cache.into_inner() { + cache.insert(k, v); + } + } else { + self.storage_cache = other.storage_cache; } + self.original_storage_cache = other.original_storage_cache; + self.storage_root = other.storage_root; self.storage_changes = other.storage_changes; } } @@ -526,7 +627,7 @@ mod tests { let mut db = MemoryDB::new(); let mut db = AccountDBMut::new(&mut db, &Address::new()); let rlp = { - let mut a = Account::new_contract(69.into(), 0.into()); + let mut a = Account::new_contract(69.into(), 0.into(), KECCAK_NULL_RLP); a.set_storage(0x00u64.into(), 0x1234u64.into()); a.commit_storage(&Default::default(), &mut db).unwrap(); a.init_code(vec![]); @@ -535,7 +636,7 @@ mod tests { }; let a = Account::from_rlp(&rlp).expect("decoding db value failed"); - assert_eq!(*a.storage_root().unwrap(), "c57e1afb758b07f8d2c8f13a3b6e44fa5ff94ab266facc5a4fd3f062426e50b2".into()); + assert_eq!(a.storage_root().unwrap(), "c57e1afb758b07f8d2c8f13a3b6e44fa5ff94ab266facc5a4fd3f062426e50b2".into()); assert_eq!(a.storage_at(&db.immutable(), &0x00u64.into()).unwrap(), 0x1234u64.into()); assert_eq!(a.storage_at(&db.immutable(), &0x01u64.into()).unwrap(), H256::default()); } @@ -546,7 +647,7 @@ mod tests { let mut db = AccountDBMut::new(&mut db, &Address::new()); let rlp = { - let mut a = Account::new_contract(69.into(), 0.into()); + let mut a = Account::new_contract(69.into(), 0.into(), KECCAK_NULL_RLP); a.init_code(vec![0x55, 0x44, 0xffu8]); a.commit_code(&mut db); a.rlp() @@ -561,18 +662,18 @@ mod tests { #[test] fn commit_storage() { - let mut a = Account::new_contract(69.into(), 0.into()); + let mut a = Account::new_contract(69.into(), 0.into(), KECCAK_NULL_RLP); let mut db = MemoryDB::new(); let mut db = AccountDBMut::new(&mut db, &Address::new()); a.set_storage(0.into(), 0x1234.into()); assert_eq!(a.storage_root(), None); a.commit_storage(&Default::default(), &mut db).unwrap(); - assert_eq!(*a.storage_root().unwrap(), "c57e1afb758b07f8d2c8f13a3b6e44fa5ff94ab266facc5a4fd3f062426e50b2".into()); + assert_eq!(a.storage_root().unwrap(), "c57e1afb758b07f8d2c8f13a3b6e44fa5ff94ab266facc5a4fd3f062426e50b2".into()); } #[test] fn commit_remove_commit_storage() { - let mut a = Account::new_contract(69.into(), 0.into()); + let mut a = Account::new_contract(69.into(), 0.into(), KECCAK_NULL_RLP); let mut db = MemoryDB::new(); let mut db = AccountDBMut::new(&mut db, &Address::new()); a.set_storage(0.into(), 0x1234.into()); @@ -581,12 +682,12 @@ mod tests { a.commit_storage(&Default::default(), &mut db).unwrap(); a.set_storage(1.into(), 0.into()); a.commit_storage(&Default::default(), &mut db).unwrap(); - assert_eq!(*a.storage_root().unwrap(), "c57e1afb758b07f8d2c8f13a3b6e44fa5ff94ab266facc5a4fd3f062426e50b2".into()); + assert_eq!(a.storage_root().unwrap(), "c57e1afb758b07f8d2c8f13a3b6e44fa5ff94ab266facc5a4fd3f062426e50b2".into()); } #[test] fn commit_code() { - let mut a = Account::new_contract(69.into(), 0.into()); + let mut a = Account::new_contract(69.into(), 0.into(), KECCAK_NULL_RLP); let mut db = MemoryDB::new(); let mut db = AccountDBMut::new(&mut db, &Address::new()); a.init_code(vec![0x55, 0x44, 0xffu8]); @@ -598,7 +699,7 @@ mod tests { #[test] fn reset_code() { - let mut a = Account::new_contract(69.into(), 0.into()); + let mut a = Account::new_contract(69.into(), 0.into(), KECCAK_NULL_RLP); let mut db = MemoryDB::new(); let mut db = AccountDBMut::new(&mut db, &Address::new()); a.init_code(vec![0x55, 0x44, 0xffu8]); @@ -629,7 +730,7 @@ mod tests { assert_eq!(*a.balance(), 69u8.into()); assert_eq!(*a.nonce(), 0u8.into()); assert_eq!(a.code_hash(), KECCAK_EMPTY); - assert_eq!(a.storage_root().unwrap(), &KECCAK_NULL_RLP); + assert_eq!(a.storage_root().unwrap(), KECCAK_NULL_RLP); } #[test] diff --git a/ethcore/src/state/mod.rs b/ethcore/src/state/mod.rs index 323e11ccb..fb843c9cc 100644 --- a/ethcore/src/state/mod.rs +++ b/ethcore/src/state/mod.rs @@ -181,6 +181,8 @@ impl AccountEntry { Some(acc) => { if let Some(ref mut ours) = self.account { ours.overwrite_with(acc); + } else { + self.account = Some(acc); } }, None => self.account = None, @@ -401,9 +403,12 @@ impl State { self.factories.vm.clone() } - /// Create a recoverable checkpoint of this state. - pub fn checkpoint(&mut self) { - self.checkpoints.get_mut().push(HashMap::new()); + /// Create a recoverable checkpoint of this state. Return the checkpoint index. + pub fn checkpoint(&mut self) -> usize { + let checkpoints = self.checkpoints.get_mut(); + let index = checkpoints.len(); + checkpoints.push(HashMap::new()); + index } /// Merge last checkpoint with previous. @@ -416,7 +421,16 @@ impl State { **prev = checkpoint; } else { for (k, v) in checkpoint.drain() { - prev.entry(k).or_insert(v); + match prev.entry(k) { + Entry::Occupied(mut e) => { + if e.get().is_none() { + e.insert(v); + } + }, + Entry::Vacant(e) => { + e.insert(v); + } + } } } } @@ -494,8 +508,10 @@ impl State { /// Create a new contract at address `contract`. If there is already an account at the address /// it will have its code reset, ready for `init_code()`. - pub fn new_contract(&mut self, contract: &Address, balance: U256, nonce_offset: U256) { - self.insert_cache(contract, AccountEntry::new_dirty(Some(Account::new_contract(balance, self.account_start_nonce + nonce_offset)))); + pub fn new_contract(&mut self, contract: &Address, balance: U256, nonce_offset: U256) -> TrieResult<()> { + let original_storage_root = self.original_storage_root(contract)?; + self.insert_cache(contract, AccountEntry::new_dirty(Some(Account::new_contract(balance, self.account_start_nonce + nonce_offset, original_storage_root)))); + Ok(()) } /// Remove an existing account. @@ -536,11 +552,84 @@ impl State { /// Get the storage root of account `a`. pub fn storage_root(&self, a: &Address) -> TrieResult> { self.ensure_cached(a, RequireCache::None, true, - |a| a.as_ref().and_then(|account| account.storage_root().cloned())) + |a| a.as_ref().and_then(|account| account.storage_root())) } - /// Mutate storage of account `address` so that it is `value` for `key`. - pub fn storage_at(&self, address: &Address, key: &H256) -> TrieResult { + /// Get the original storage root since last commit of account `a`. + pub fn original_storage_root(&self, a: &Address) -> TrieResult { + Ok(self.ensure_cached(a, RequireCache::None, true, + |a| a.as_ref().map(|account| account.original_storage_root()))? + .unwrap_or(KECCAK_NULL_RLP)) + } + + /// Get the value of storage at a specific checkpoint. + pub fn checkpoint_storage_at(&self, checkpoint_index: usize, address: &Address, key: &H256) -> TrieResult> { + enum ReturnKind { + /// Use original storage at value at this address. + OriginalAt, + /// Use the downward checkpoint value. + Downward, + } + + let (checkpoints_len, kind) = { + let checkpoints = self.checkpoints.borrow(); + let checkpoints_len = checkpoints.len(); + let checkpoint = match checkpoints.get(checkpoint_index) { + Some(checkpoint) => checkpoint, + // The checkpoint was not found. Return None. + None => return Ok(None), + }; + + let kind = match checkpoint.get(address) { + // The account exists at this checkpoint. + Some(Some(AccountEntry { account: Some(ref account), .. })) => { + if let Some(value) = account.cached_storage_at(key) { + return Ok(Some(value)); + } else { + // This account has checkpoint entry, but the key is not in the entry's cache. We can use + // original_storage_at if current account's original storage root is the same as checkpoint + // account's original storage root. Otherwise, the account must be a newly created contract. + if account.base_storage_root() == self.original_storage_root(address)? { + ReturnKind::OriginalAt + } else { + // If account base storage root is different from the original storage root since last + // commit, then it can only be created from a new contract, where the base storage root + // would always be empty. Note that this branch is actually never called, because + // `cached_storage_at` handled this case. + warn!("Trying to get an account's cached storage value, but base storage root does not equal to original storage root! Assuming the value is empty."); + return Ok(Some(H256::new())); + } + } + }, + // The account didn't exist at that point. Return empty value. + Some(Some(AccountEntry { account: None, .. })) => return Ok(Some(H256::new())), + // The value was not cached at that checkpoint, meaning it was not modified at all. + Some(None) => ReturnKind::OriginalAt, + // This key does not have a checkpoint entry. + None => ReturnKind::Downward, + }; + + (checkpoints_len, kind) + }; + + match kind { + ReturnKind::Downward => { + if checkpoint_index >= checkpoints_len - 1 { + Ok(Some(self.storage_at(address, key)?)) + } else { + self.checkpoint_storage_at(checkpoint_index + 1, address, key) + } + }, + ReturnKind::OriginalAt => Ok(Some(self.original_storage_at(address, key)?)), + } + } + + fn storage_at_inner( + &self, address: &Address, key: &H256, f_cached_at: FCachedStorageAt, f_at: FStorageAt, + ) -> TrieResult where + FCachedStorageAt: Fn(&Account, &H256) -> Option, + FStorageAt: Fn(&Account, &HashDB, &H256) -> TrieResult + { // Storage key search and update works like this: // 1. If there's an entry for the account in the local cache check for the key and return it if found. // 2. If there's an entry for the account in the global cache check for the key or load it into that account. @@ -553,7 +642,7 @@ impl State { if let Some(maybe_acc) = local_cache.get(address) { match maybe_acc.account { Some(ref account) => { - if let Some(value) = account.cached_storage_at(key) { + if let Some(value) = f_cached_at(account, key) { return Ok(value); } else { local_account = Some(maybe_acc); @@ -567,7 +656,7 @@ impl State { None => Ok(H256::new()), Some(a) => { let account_db = self.factories.accountdb.readonly(self.db.as_hashdb(), a.address_hash(address)); - a.storage_at(account_db.as_hashdb(), key) + f_at(a, account_db.as_hashdb(), key) } }); @@ -579,7 +668,7 @@ impl State { if let Some(ref mut acc) = local_account { if let Some(ref account) = acc.account { let account_db = self.factories.accountdb.readonly(self.db.as_hashdb(), account.address_hash(address)); - return account.storage_at(account_db.as_hashdb(), key) + return f_at(account, account_db.as_hashdb(), key) } else { return Ok(H256::new()) } @@ -595,12 +684,32 @@ impl State { let maybe_acc = db.get_with(address, from_rlp)?; let r = maybe_acc.as_ref().map_or(Ok(H256::new()), |a| { let account_db = self.factories.accountdb.readonly(self.db.as_hashdb(), a.address_hash(address)); - a.storage_at(account_db.as_hashdb(), key) + f_at(a, account_db.as_hashdb(), key) }); self.insert_cache(address, AccountEntry::new_clean(maybe_acc)); r } + /// Mutate storage of account `address` so that it is `value` for `key`. + pub fn storage_at(&self, address: &Address, key: &H256) -> TrieResult { + self.storage_at_inner( + address, + key, + |account, key| { account.cached_storage_at(key) }, + |account, db, key| { account.storage_at(db, key) }, + ) + } + + /// Get the value of storage after last state commitment. + pub fn original_storage_at(&self, address: &Address, key: &H256) -> TrieResult { + self.storage_at_inner( + address, + key, + |account, key| { account.cached_original_storage_at(key) }, + |account, db, key| { account.original_storage_at(db, key) }, + ) + } + /// Get accounts' code. pub fn code(&self, a: &Address) -> TrieResult>> { self.ensure_cached(a, RequireCache::Code, true, @@ -671,13 +780,13 @@ impl State { /// Initialise the code of account `a` so that it is `code`. /// NOTE: Account should have been created with `new_contract`. pub fn init_code(&mut self, a: &Address, code: Bytes) -> TrieResult<()> { - self.require_or_from(a, true, || Account::new_contract(0.into(), self.account_start_nonce), |_|{})?.init_code(code); + self.require_or_from(a, true, || Account::new_contract(0.into(), self.account_start_nonce, KECCAK_NULL_RLP), |_| {})?.init_code(code); Ok(()) } /// Reset the code of account `a` so that it is `code`. pub fn reset_code(&mut self, a: &Address, code: Bytes) -> TrieResult<()> { - self.require_or_from(a, true, || Account::new_contract(0.into(), self.account_start_nonce), |_|{})?.reset_code(code); + self.require_or_from(a, true, || Account::new_contract(0.into(), self.account_start_nonce, KECCAK_NULL_RLP), |_| {})?.reset_code(code); Ok(()) } @@ -761,6 +870,7 @@ impl State { /// Commits our cached account changes into the trie. pub fn commit(&mut self) -> Result<(), Error> { + assert!(self.checkpoints.borrow().is_empty()); // first, commit the sub trees. let mut accounts = self.cache.borrow_mut(); for (address, ref mut a) in accounts.iter_mut().filter(|&(_, ref a)| a.is_dirty()) { @@ -806,6 +916,7 @@ impl State { /// Clear state cache pub fn clear(&mut self) { + assert!(self.checkpoints.borrow().is_empty()); self.cache.borrow_mut().clear(); } @@ -1000,7 +1111,7 @@ impl State { /// Pull account `a` in our cache from the trie DB. `require_code` requires that the code be cached, too. fn require<'a>(&'a self, a: &Address, require_code: bool) -> TrieResult> { - self.require_or_from(a, require_code, || Account::new_basic(0u8.into(), self.account_start_nonce), |_|{}) + self.require_or_from(a, require_code, || Account::new_basic(0u8.into(), self.account_start_nonce), |_| {}) } /// Pull account `a` in our cache from the trie DB. `require_code` requires that the code be cached, too. @@ -1140,7 +1251,7 @@ mod tests { use std::sync::Arc; use std::str::FromStr; use rustc_hex::FromHex; - use hash::keccak; + use hash::{keccak, KECCAK_NULL_RLP}; use super::*; use ethkey::Secret; use ethereum_types::{H256, U256, Address}; @@ -1992,7 +2103,7 @@ mod tests { let a = Address::zero(); let (root, db) = { let mut state = get_temp_state(); - state.require_or_from(&a, false, ||Account::new_contract(42.into(), 0.into()), |_|{}).unwrap(); + state.require_or_from(&a, false, || Account::new_contract(42.into(), 0.into(), KECCAK_NULL_RLP), |_|{}).unwrap(); state.init_code(&a, vec![1, 2, 3]).unwrap(); assert_eq!(state.code(&a).unwrap(), Some(Arc::new(vec![1u8, 2, 3]))); state.commit().unwrap(); @@ -2196,6 +2307,161 @@ mod tests { assert_eq!(state.balance(&a).unwrap(), U256::from(0)); } + #[test] + fn checkpoint_revert_to_get_storage_at() { + let mut state = get_temp_state(); + let a = Address::zero(); + let k = H256::from(U256::from(0)); + + let c0 = state.checkpoint(); + let c1 = state.checkpoint(); + state.set_storage(&a, k, H256::from(U256::from(1))).unwrap(); + + assert_eq!(state.checkpoint_storage_at(c0, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.checkpoint_storage_at(c1, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.storage_at(&a, &k).unwrap(), H256::from(U256::from(1))); + + state.revert_to_checkpoint(); // Revert to c1. + assert_eq!(state.checkpoint_storage_at(c0, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.storage_at(&a, &k).unwrap(), H256::from(U256::from(0))); + } + + #[test] + fn checkpoint_from_empty_get_storage_at() { + let mut state = get_temp_state(); + let a = Address::zero(); + let k = H256::from(U256::from(0)); + let k2 = H256::from(U256::from(1)); + + assert_eq!(state.storage_at(&a, &k).unwrap(), H256::from(U256::from(0))); + state.clear(); + + let c0 = state.checkpoint(); + state.new_contract(&a, U256::zero(), U256::zero()).unwrap(); + let c1 = state.checkpoint(); + state.set_storage(&a, k, H256::from(U256::from(1))).unwrap(); + let c2 = state.checkpoint(); + let c3 = state.checkpoint(); + state.set_storage(&a, k2, H256::from(U256::from(3))).unwrap(); + state.set_storage(&a, k, H256::from(U256::from(3))).unwrap(); + let c4 = state.checkpoint(); + state.set_storage(&a, k, H256::from(U256::from(4))).unwrap(); + let c5 = state.checkpoint(); + + assert_eq!(state.checkpoint_storage_at(c0, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.checkpoint_storage_at(c1, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.checkpoint_storage_at(c2, &a, &k).unwrap(), Some(H256::from(U256::from(1)))); + assert_eq!(state.checkpoint_storage_at(c3, &a, &k).unwrap(), Some(H256::from(U256::from(1)))); + assert_eq!(state.checkpoint_storage_at(c4, &a, &k).unwrap(), Some(H256::from(U256::from(3)))); + assert_eq!(state.checkpoint_storage_at(c5, &a, &k).unwrap(), Some(H256::from(U256::from(4)))); + + state.discard_checkpoint(); // Commit/discard c5. + assert_eq!(state.checkpoint_storage_at(c0, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.checkpoint_storage_at(c1, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.checkpoint_storage_at(c2, &a, &k).unwrap(), Some(H256::from(U256::from(1)))); + assert_eq!(state.checkpoint_storage_at(c3, &a, &k).unwrap(), Some(H256::from(U256::from(1)))); + assert_eq!(state.checkpoint_storage_at(c4, &a, &k).unwrap(), Some(H256::from(U256::from(3)))); + + state.revert_to_checkpoint(); // Revert to c4. + assert_eq!(state.checkpoint_storage_at(c0, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.checkpoint_storage_at(c1, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.checkpoint_storage_at(c2, &a, &k).unwrap(), Some(H256::from(U256::from(1)))); + assert_eq!(state.checkpoint_storage_at(c3, &a, &k).unwrap(), Some(H256::from(U256::from(1)))); + + state.discard_checkpoint(); // Commit/discard c3. + assert_eq!(state.checkpoint_storage_at(c0, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.checkpoint_storage_at(c1, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.checkpoint_storage_at(c2, &a, &k).unwrap(), Some(H256::from(U256::from(1)))); + + state.revert_to_checkpoint(); // Revert to c2. + assert_eq!(state.checkpoint_storage_at(c0, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.checkpoint_storage_at(c1, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + + state.discard_checkpoint(); // Commit/discard c1. + assert_eq!(state.checkpoint_storage_at(c0, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + } + + #[test] + fn checkpoint_get_storage_at() { + let mut state = get_temp_state(); + let a = Address::zero(); + let k = H256::from(U256::from(0)); + let k2 = H256::from(U256::from(1)); + + state.set_storage(&a, k, H256::from(U256::from(0xffff))).unwrap(); + state.commit().unwrap(); + state.clear(); + + assert_eq!(state.storage_at(&a, &k).unwrap(), H256::from(U256::from(0xffff))); + state.clear(); + + let cm1 = state.checkpoint(); + let c0 = state.checkpoint(); + state.new_contract(&a, U256::zero(), U256::zero()).unwrap(); + let c1 = state.checkpoint(); + state.set_storage(&a, k, H256::from(U256::from(1))).unwrap(); + let c2 = state.checkpoint(); + let c3 = state.checkpoint(); + state.set_storage(&a, k2, H256::from(U256::from(3))).unwrap(); + state.set_storage(&a, k, H256::from(U256::from(3))).unwrap(); + let c4 = state.checkpoint(); + state.set_storage(&a, k, H256::from(U256::from(4))).unwrap(); + let c5 = state.checkpoint(); + + assert_eq!(state.checkpoint_storage_at(cm1, &a, &k).unwrap(), Some(H256::from(U256::from(0xffff)))); + assert_eq!(state.checkpoint_storage_at(c0, &a, &k).unwrap(), Some(H256::from(U256::from(0xffff)))); + assert_eq!(state.checkpoint_storage_at(c1, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.checkpoint_storage_at(c2, &a, &k).unwrap(), Some(H256::from(U256::from(1)))); + assert_eq!(state.checkpoint_storage_at(c3, &a, &k).unwrap(), Some(H256::from(U256::from(1)))); + assert_eq!(state.checkpoint_storage_at(c4, &a, &k).unwrap(), Some(H256::from(U256::from(3)))); + assert_eq!(state.checkpoint_storage_at(c5, &a, &k).unwrap(), Some(H256::from(U256::from(4)))); + + state.discard_checkpoint(); // Commit/discard c5. + assert_eq!(state.checkpoint_storage_at(cm1, &a, &k).unwrap(), Some(H256::from(U256::from(0xffff)))); + assert_eq!(state.checkpoint_storage_at(c0, &a, &k).unwrap(), Some(H256::from(U256::from(0xffff)))); + assert_eq!(state.checkpoint_storage_at(c1, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.checkpoint_storage_at(c2, &a, &k).unwrap(), Some(H256::from(U256::from(1)))); + assert_eq!(state.checkpoint_storage_at(c3, &a, &k).unwrap(), Some(H256::from(U256::from(1)))); + assert_eq!(state.checkpoint_storage_at(c4, &a, &k).unwrap(), Some(H256::from(U256::from(3)))); + + state.revert_to_checkpoint(); // Revert to c4. + assert_eq!(state.checkpoint_storage_at(cm1, &a, &k).unwrap(), Some(H256::from(U256::from(0xffff)))); + assert_eq!(state.checkpoint_storage_at(c0, &a, &k).unwrap(), Some(H256::from(U256::from(0xffff)))); + assert_eq!(state.checkpoint_storage_at(c1, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.checkpoint_storage_at(c2, &a, &k).unwrap(), Some(H256::from(U256::from(1)))); + assert_eq!(state.checkpoint_storage_at(c3, &a, &k).unwrap(), Some(H256::from(U256::from(1)))); + + state.discard_checkpoint(); // Commit/discard c3. + assert_eq!(state.checkpoint_storage_at(cm1, &a, &k).unwrap(), Some(H256::from(U256::from(0xffff)))); + assert_eq!(state.checkpoint_storage_at(c0, &a, &k).unwrap(), Some(H256::from(U256::from(0xffff)))); + assert_eq!(state.checkpoint_storage_at(c1, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + assert_eq!(state.checkpoint_storage_at(c2, &a, &k).unwrap(), Some(H256::from(U256::from(1)))); + + state.revert_to_checkpoint(); // Revert to c2. + assert_eq!(state.checkpoint_storage_at(cm1, &a, &k).unwrap(), Some(H256::from(U256::from(0xffff)))); + assert_eq!(state.checkpoint_storage_at(c0, &a, &k).unwrap(), Some(H256::from(U256::from(0xffff)))); + assert_eq!(state.checkpoint_storage_at(c1, &a, &k).unwrap(), Some(H256::from(U256::from(0)))); + + state.discard_checkpoint(); // Commit/discard c1. + assert_eq!(state.checkpoint_storage_at(cm1, &a, &k).unwrap(), Some(H256::from(U256::from(0xffff)))); + assert_eq!(state.checkpoint_storage_at(c0, &a, &k).unwrap(), Some(H256::from(U256::from(0xffff)))); + } + + #[test] + fn kill_account_with_checkpoints() { + let mut state = get_temp_state(); + let a = Address::zero(); + let k = H256::from(U256::from(0)); + state.checkpoint(); + state.set_storage(&a, k, H256::from(U256::from(1))).unwrap(); + state.checkpoint(); + state.kill_account(&a); + + assert_eq!(state.storage_at(&a, &k).unwrap(), H256::from(U256::from(0))); + state.revert_to_checkpoint(); + assert_eq!(state.storage_at(&a, &k).unwrap(), H256::from(U256::from(1))); + } + #[test] fn create_empty() { let mut state = get_temp_state(); @@ -2233,7 +2499,7 @@ mod tests { state.add_balance(&b, &100.into(), CleanupMode::ForceCreate).unwrap(); // create a dust account state.add_balance(&c, &101.into(), CleanupMode::ForceCreate).unwrap(); // create a normal account state.add_balance(&d, &99.into(), CleanupMode::ForceCreate).unwrap(); // create another dust account - state.new_contract(&e, 100.into(), 1.into()); // create a contract account + state.new_contract(&e, 100.into(), 1.into()).unwrap(); // create a contract account state.init_code(&e, vec![0x00]).unwrap(); state.commit().unwrap(); state.drop() diff --git a/ethcore/src/state/substate.rs b/ethcore/src/state/substate.rs index c2f3c62dc..054b8b384 100644 --- a/ethcore/src/state/substate.rs +++ b/ethcore/src/state/substate.rs @@ -34,8 +34,8 @@ pub struct Substate { /// Any logs. pub logs: Vec, - /// Refund counter of SSTORE nonzero -> zero. - pub sstore_clears_count: U256, + /// Refund counter of SSTORE. + pub sstore_clears_refund: U256, /// Created contracts. pub contracts_created: Vec
, @@ -52,7 +52,7 @@ impl Substate { self.suicides.extend(s.suicides); self.touched.extend(s.touched); self.logs.extend(s.logs); - self.sstore_clears_count = self.sstore_clears_count + s.sstore_clears_count; + self.sstore_clears_refund = self.sstore_clears_refund + s.sstore_clears_refund; self.contracts_created.extend(s.contracts_created); } @@ -86,7 +86,7 @@ mod tests { topics: vec![], data: vec![] }); - sub_state.sstore_clears_count = 5.into(); + sub_state.sstore_clears_refund = (15000 * 5).into(); sub_state.suicides.insert(10u64.into()); let mut sub_state_2 = Substate::new(); @@ -96,11 +96,11 @@ mod tests { topics: vec![], data: vec![] }); - sub_state_2.sstore_clears_count = 7.into(); + sub_state_2.sstore_clears_refund = (15000 * 7).into(); sub_state.accrue(sub_state_2); assert_eq!(sub_state.contracts_created.len(), 2); - assert_eq!(sub_state.sstore_clears_count, 12.into()); + assert_eq!(sub_state.sstore_clears_refund, (15000 * 12).into()); assert_eq!(sub_state.suicides.len(), 1); } } diff --git a/ethcore/vm/src/ext.rs b/ethcore/vm/src/ext.rs index d95573c3c..168640593 100644 --- a/ethcore/vm/src/ext.rs +++ b/ethcore/vm/src/ext.rs @@ -63,6 +63,9 @@ pub enum CreateContractAddress { /// Externalities interface for EVMs pub trait Ext { + /// Returns the storage value for a given key if reversion happens on the current transaction. + fn initial_storage_at(&self, key: &H256) -> Result; + /// Returns a value for given key. fn storage_at(&self, key: &H256) -> Result; @@ -136,8 +139,11 @@ pub trait Ext { /// then A depth is 0, B is 1, C is 2 and so on. fn depth(&self) -> usize; - /// Increments sstore refunds count by 1. - fn inc_sstore_clears(&mut self); + /// Increments sstore refunds counter. + fn add_sstore_refund(&mut self, value: U256); + + /// Decrements sstore refunds counter. + fn sub_sstore_refund(&mut self, value: U256); /// Decide if any more operations should be traced. Passthrough for the VM trace. fn trace_next_instruction(&mut self, _pc: usize, _instruction: u8, _current_gas: U256) -> bool { false } diff --git a/ethcore/vm/src/schedule.rs b/ethcore/vm/src/schedule.rs index b3d71457e..462e96b32 100644 --- a/ethcore/vm/src/schedule.rs +++ b/ethcore/vm/src/schedule.rs @@ -117,6 +117,8 @@ pub struct Schedule { pub have_bitwise_shifting: bool, /// Kill basic accounts below this balance if touched. pub kill_dust: CleanDustMode, + /// Enable EIP-1283 rules + pub eip1283: bool, /// VM execution does not increase null signed address nonce if this field is true. pub keep_unsigned_nonce: bool, /// Wasm extra schedule settings, if wasm activated @@ -250,6 +252,7 @@ impl Schedule { blockhash_gas: 20, have_static_call: false, kill_dust: CleanDustMode::Off, + eip1283: false, keep_unsigned_nonce: false, wasm: None, } @@ -323,6 +326,7 @@ impl Schedule { blockhash_gas: 20, have_static_call: false, kill_dust: CleanDustMode::Off, + eip1283: false, keep_unsigned_nonce: false, wasm: None, } diff --git a/ethcore/vm/src/tests.rs b/ethcore/vm/src/tests.rs index 3dac74484..b6e44f000 100644 --- a/ethcore/vm/src/tests.rs +++ b/ethcore/vm/src/tests.rs @@ -56,7 +56,7 @@ pub struct FakeExt { pub store: HashMap, pub suicides: HashSet
, pub calls: HashSet, - pub sstore_clears: usize, + pub sstore_clears: U256, pub depth: usize, pub blockhashes: HashMap, pub codes: HashMap>, @@ -105,6 +105,10 @@ impl FakeExt { } impl Ext for FakeExt { + fn initial_storage_at(&self, _key: &H256) -> Result { + Ok(H256::new()) + } + fn storage_at(&self, key: &H256) -> Result { Ok(self.store.get(key).unwrap_or(&H256::new()).clone()) } @@ -216,8 +220,12 @@ impl Ext for FakeExt { self.is_static } - fn inc_sstore_clears(&mut self) { - self.sstore_clears += 1; + fn add_sstore_refund(&mut self, value: U256) { + self.sstore_clears = self.sstore_clears + value; + } + + fn sub_sstore_refund(&mut self, value: U256) { + self.sstore_clears = self.sstore_clears - value; } fn trace_next_instruction(&mut self, _pc: usize, _instruction: u8, _gas: U256) -> bool { diff --git a/ethcore/wasm/src/runtime.rs b/ethcore/wasm/src/runtime.rs index 3c5d27d5c..41c0d950d 100644 --- a/ethcore/wasm/src/runtime.rs +++ b/ethcore/wasm/src/runtime.rs @@ -285,7 +285,8 @@ impl<'a> Runtime<'a> { self.ext.set_storage(key, val).map_err(|_| Error::StorageUpdateError)?; if former_val != H256::zero() && val == H256::zero() { - self.ext.inc_sstore_clears(); + let sstore_clears_schedule = U256::from(self.schedule().sstore_refund_gas); + self.ext.add_sstore_refund(sstore_clears_schedule); } Ok(()) diff --git a/json/src/spec/params.rs b/json/src/spec/params.rs index 4210bbe45..ea0a55d49 100644 --- a/json/src/spec/params.rs +++ b/json/src/spec/params.rs @@ -109,6 +109,8 @@ pub struct Params { #[serde(rename="eip1052Transition")] pub eip1052_transition: Option, /// See `CommonParams` docs. + #[serde(rename="eip1283Transition")] + pub eip1283_transition: Option, #[serde(rename="eip1014Transition")] pub eip1014_transition: Option, /// See `CommonParams` docs.