From 4581469e780498b6e179e6ccdde6d3a0f6daa040 Mon Sep 17 00:00:00 2001
From: Robert Habermeier <rphmeier@gmail.com>
Date: Thu, 13 Oct 2016 23:28:56 +0200
Subject: [PATCH] mitigate refcell conflict in state diffing (#2601)

* mitigate refcell conflict in state diffing

Also uses RefCell::get_mut in a few places.

* Add test case
---
 ethcore/src/state/mod.rs | 44 +++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/ethcore/src/state/mod.rs b/ethcore/src/state/mod.rs
index 39c8bbc11..6e34e9367 100644
--- a/ethcore/src/state/mod.rs
+++ b/ethcore/src/state/mod.rs
@@ -239,15 +239,15 @@ impl State {
 
 	/// Create a recoverable snaphot of this state.
 	pub fn snapshot(&mut self) {
-		self.snapshots.borrow_mut().push(HashMap::new());
+		self.snapshots.get_mut().push(HashMap::new());
 	}
 
 	/// Merge last snapshot with previous.
 	pub fn discard_snapshot(&mut self) {
 		// merge with previous snapshot
-		let last = self.snapshots.borrow_mut().pop();
+		let last = self.snapshots.get_mut().pop();
 		if let Some(mut snapshot) = last {
-			if let Some(ref mut prev) = self.snapshots.borrow_mut().last_mut() {
+			if let Some(ref mut prev) = self.snapshots.get_mut().last_mut() {
 				if prev.is_empty() {
 					**prev = snapshot;
 				} else {
@@ -261,11 +261,11 @@ impl State {
 
 	/// Revert to the last snapshot and discard it.
 	pub fn revert_to_snapshot(&mut self) {
-		if let Some(mut snapshot) = self.snapshots.borrow_mut().pop() {
+		if let Some(mut snapshot) = self.snapshots.get_mut().pop() {
 			for (k, v) in snapshot.drain() {
 				match v {
 					Some(v) => {
-						match self.cache.borrow_mut().entry(k) {
+						match self.cache.get_mut().entry(k) {
 							Entry::Occupied(mut e) => {
 								// Merge snapshotted changes back into the main account
 								// storage preserving the cache.
@@ -277,7 +277,7 @@ impl State {
 						}
 					},
 					None => {
-						match self.cache.borrow_mut().entry(k) {
+						match self.cache.get_mut().entry(k) {
 							Entry::Occupied(e) => {
 								if e.get().is_dirty() {
 									e.remove();
@@ -578,14 +578,14 @@ impl State {
 	}
 
 	fn query_pod(&mut self, query: &PodState) {
-		for (address, pod_account) in query.get() {
-			self.ensure_cached(address, RequireCache::Code, |a| {
-				if a.is_some() {
-					for key in pod_account.storage.keys() {
-						self.storage_at(address, key);
-					}
-				}
-			});
+		for (address, pod_account) in query.get().into_iter()
+			.filter(|&(ref a, _)| self.ensure_cached(a, RequireCache::Code, |a| a.is_some()))
+		{
+			// needs to be split into two parts for the refcell code here
+			// to work.
+			for key in pod_account.storage.keys() {
+				self.storage_at(address, key);
+			}
 		}
 	}
 
@@ -1797,4 +1797,20 @@ fn create_empty() {
 	assert_eq!(state.root().hex(), "56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421");
 }
 
+#[test]
+fn should_not_panic_on_state_diff_with_storage() {
+	let state = get_temp_state();
+	let mut state = state.reference().clone();
+
+	let a: Address = 0xa.into();
+	state.init_code(&a, b"abcdefg".to_vec());
+	state.add_balance(&a, &256.into());
+	state.set_storage(&a, 0xb.into(), 0xc.into());
+
+	let mut new_state = state.clone();
+	new_state.set_storage(&a, 0xb.into(), 0xd.into());
+
+	new_state.diff_from(state);
+}
+
 }