diff --git a/ethcore/account-state/src/state.rs b/ethcore/account-state/src/state.rs index 13086d7b1..b7e02eef5 100644 --- a/ethcore/account-state/src/state.rs +++ b/ethcore/account-state/src/state.rs @@ -352,7 +352,7 @@ impl State { fn insert_cache(&self, address: &Address, account: AccountEntry) { // Dirty account which is not in the cache means this is a new account. - // It goes directly into the checkpoint as there's nothing to rever to. + // It goes directly into the checkpoint as there's nothing to revert to. // // In all other cases account is read as clean first, and after that made // dirty in and added to the checkpoint with `note_cache`. @@ -759,20 +759,21 @@ impl State { } /// Remove any touched empty or dust accounts. - pub fn kill_garbage(&mut self, touched: &HashSet
, remove_empty_touched: bool, min_balance: &Option, kill_contracts: bool) -> TrieResult<()> { - let to_kill: HashSet<_> = { - self.cache.borrow().iter().filter_map(|(address, ref entry)| - if touched.contains(address) && // Check all touched accounts - ((remove_empty_touched && entry.exists_and_is_null()) // Remove all empty touched accounts. + pub fn kill_garbage(&mut self, touched: &HashSet
, min_balance: &Option, kill_contracts: bool) -> TrieResult<()> { + let to_kill: HashSet<_> = + touched.iter().filter_map(|address| { // Check all touched accounts + self.cache.borrow().get(address).and_then(|entry| { + if entry.exists_and_is_null() // Remove all empty touched accounts. || min_balance.map_or(false, |ref balance| entry.account.as_ref().map_or(false, |account| - (account.is_basic() || kill_contracts) // Remove all basic and optionally contract accounts where balance has been decreased. - && account.balance() < balance && entry.old_balance.as_ref().map_or(false, |b| account.balance() < b)))) { + (account.is_basic() || kill_contracts) // Remove all basic and optionally contract accounts where balance has been decreased. + && account.balance() < balance && entry.old_balance.as_ref().map_or(false, |b| account.balance() < b))) { + Some(address) + } else { None } + }) + }).collect(); - Some(address.clone()) - } else { None }).collect() - }; for address in to_kill { - self.kill_account(&address); + self.kill_account(address) } Ok(()) } diff --git a/ethcore/executive-state/src/lib.rs b/ethcore/executive-state/src/lib.rs index f41dba443..0751aac8c 100644 --- a/ethcore/executive-state/src/lib.rs +++ b/ethcore/executive-state/src/lib.rs @@ -1573,15 +1573,15 @@ mod tests { state.transfer_balance(&b, &x, &1.into(), CleanupMode::TrackTouched(&mut touched)).unwrap(); // touch an account decreasing its balance state.transfer_balance(&c, &x, &1.into(), CleanupMode::TrackTouched(&mut touched)).unwrap(); // touch an account decreasing its balance state.transfer_balance(&e, &x, &1.into(), CleanupMode::TrackTouched(&mut touched)).unwrap(); // touch an account decreasing its balance - state.kill_garbage(&touched, true, &None, false).unwrap(); + state.kill_garbage(&touched, &None, false).unwrap(); assert!(!state.exists(&a).unwrap()); assert!(state.exists(&b).unwrap()); - state.kill_garbage(&touched, true, &Some(100.into()), false).unwrap(); + state.kill_garbage(&touched,&Some(100.into()), false).unwrap(); assert!(!state.exists(&b).unwrap()); assert!(state.exists(&c).unwrap()); assert!(state.exists(&d).unwrap()); assert!(state.exists(&e).unwrap()); - state.kill_garbage(&touched, true, &Some(100.into()), true).unwrap(); + state.kill_garbage(&touched, &Some(100.into()), true).unwrap(); assert!(state.exists(&c).unwrap()); assert!(state.exists(&d).unwrap()); assert!(!state.exists(&e).unwrap()); diff --git a/ethcore/machine/src/executive.rs b/ethcore/machine/src/executive.rs index 740075346..131817222 100644 --- a/ethcore/machine/src/executive.rs +++ b/ethcore/machine/src/executive.rs @@ -1175,9 +1175,17 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { } // perform garbage-collection - let min_balance = if schedule.kill_dust != CleanDustMode::Off { Some(U256::from(schedule.tx_gas).overflowing_mul(t.gas_price).0) } else { None }; - self.state.kill_garbage(&substate.touched, schedule.kill_empty, &min_balance, schedule.kill_dust == CleanDustMode::WithCodeAndStorage)?; - + if schedule.kill_empty { + let (min_balance, kill_contracts) = if schedule.kill_dust != CleanDustMode::Off { + ( + Some(U256::from(schedule.tx_gas).overflowing_mul(t.gas_price).0), + schedule.kill_dust == CleanDustMode::WithCodeAndStorage, + ) + } else { + (None, false) + }; + self.state.kill_garbage(&substate.touched, &min_balance, kill_contracts)?; + } match result { Err(vm::Error::Internal(msg)) => Err(ExecutionError::Internal(msg)), Err(exception) => { @@ -1189,9 +1197,9 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { cumulative_gas_used: self.info.gas_used + t.gas, logs: vec![], contracts_created: vec![], - output: output, - trace: trace, - vm_trace: vm_trace, + output, + trace, + vm_trace, state_diff: None, }) }, diff --git a/ethcore/src/test_helpers/evm_test_client.rs b/ethcore/src/test_helpers/evm_test_client.rs index 8769af2e8..1f2258280 100644 --- a/ethcore/src/test_helpers/evm_test_client.rs +++ b/ethcore/src/test_helpers/evm_test_client.rs @@ -286,12 +286,13 @@ impl<'a> EvmTestClient<'a> { }).ok(); // Touching also means that we should remove the account if it's within eip161 // conditions. - self.state.kill_garbage( - &vec![env_info.author].into_iter().collect(), - schedule.kill_empty, - &None, - false - ).ok(); + if schedule.kill_empty { + self.state.kill_garbage( + &vec![env_info.author].into_iter().collect(), + &None, + false + ).ok(); + } self.state.commit().ok();