Fix state cache for cumulus

client/db/src/storage_cache.rs

@@ -16,7 +16,9 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
-//! Global cache state.
+//! Global state cache. Maintains recently queried/committed state values
+//! Tracks changes over the span of a few recent blocks and handles forks
+//! by tracking/removing cache entries for conflicting changes.
 
 use std::collections::{VecDeque, HashSet, HashMap};
 use std::sync::Arc;
@@ -343,12 +345,27 @@ impl<B: BlockT> CacheChanges<B> {
 		);
 		let cache = &mut *cache;
 		// Filter out committing block if any.
-		let enacted: Vec<_> = enacted
+		let mut enacted: Vec<_> = enacted
 			.iter()
 			.filter(|h| commit_hash.as_ref().map_or(true, |p| *h != p))
 			.cloned()
 			.collect();
-		cache.sync(&enacted, retracted);
+
+		let mut retracted = std::borrow::Cow::Borrowed(retracted);
+		if let Some(commit_hash) = &commit_hash {
+			if let Some(m) = cache.modifications.iter_mut().find(|m| &m.hash == commit_hash) {
+				if m.is_canon != is_best {
+					// Same block comitted twice with different state changes.
+					// Treat it as reenacted/retracted.
+					if is_best {
+						enacted.push(commit_hash.clone());
+					} else {
+						retracted.to_mut().push(commit_hash.clone());
+					}
+				}
+			}
+		}
+		cache.sync(&enacted, &retracted);
 		// Propagate cache only if committing on top of the latest canonical state
 		// blocks are ordered by number and only one block with a given number is marked as canonical
 		// (contributed to canonical state cache)
@@ -1316,6 +1333,71 @@ mod tests {
 		);
 		assert_eq!(s.storage(&key).unwrap(), None);
 	}
+
+	#[test]
+	fn same_block_no_changes() {
+		sp_tracing::try_init_simple();
+
+		let root_parent = H256::random();
+		let key = H256::random()[..].to_vec();
+		let h1 = H256::random();
+		let h2 = H256::random();
+
+		let shared = new_shared_cache::<Block>(256*1024, (0,1));
+
+		let mut s = CachingState::new(
+			InMemoryBackend::<BlakeTwo256>::default(),
+			shared.clone(),
+			Some(root_parent),
+		);
+		s.cache.sync_cache(
+			&[],
+			&[],
+			vec![(key.clone(), Some(vec![1]))],
+			vec![],
+			Some(h1),
+			Some(1),
+			true,
+		);
+		assert_eq!(shared.lock().lru_storage.get(&key).unwrap(), &Some(vec![1]));
+
+		let mut s = CachingState::new(
+			InMemoryBackend::<BlakeTwo256>::default(),
+			shared.clone(),
+			Some(h1),
+		);
+
+		// commit as non-best
+		s.cache.sync_cache(
+			&[],
+			&[],
+			vec![(key.clone(), Some(vec![2]))],
+			vec![],
+			Some(h2),
+			Some(2),
+			false,
+		);
+
+		assert_eq!(shared.lock().lru_storage.get(&key).unwrap(), &Some(vec![1]));
+
+		let mut s = CachingState::new(
+			InMemoryBackend::<BlakeTwo256>::default(),
+			shared.clone(),
+			Some(h1),
+		);
+
+		// commit again as best with no changes
+		s.cache.sync_cache(
+			&[],
+			&[],
+			vec![],
+			vec![],
+			Some(h2),
+			Some(2),
+			true,
+		);
+		assert_eq!(s.storage(&key).unwrap(), None);
+	}
 }
 
 #[cfg(test)]