From 3a052bab38679d2a18e1f5c394efeff8601a8347 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Mon, 13 Mar 2023 17:43:08 +0100 Subject: [PATCH] Use saturating increments for Stats fields --- CHANGELOG.md | 3 +++ packages/vm/src/cache.rs | 20 ++++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 481d980afb..a9407f0864 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to ## [Unreleased] +- cosmwasm-vm: Use saturating increments for `Stats` fields to ensure we don't + run into overflow issues. + ## [1.2.2] - 2023-03-08 ### Added diff --git a/packages/vm/src/cache.rs b/packages/vm/src/cache.rs index 263979f0ca..3984e5e114 100644 --- a/packages/vm/src/cache.rs +++ b/packages/vm/src/cache.rs @@ -25,6 +25,13 @@ const CACHE_DIR: &str = "cache"; // Cacheable things. const MODULES_DIR: &str = "modules"; +/// Statistics about the usage of a cache instance. Those values are node +/// specific and must not be used in a consensus critical context. +/// When a node is hit by a client for simulations or other queries, hits and misses +/// increase. Also a node restart will reset the values. +/// +/// All values should be increment using saturated addition to ensure the node does not +/// crash in case the stats exceed the integer limit. #[derive(Debug, Default, Clone, Copy)] pub struct Stats { pub hits_pinned_memory_cache: u32, @@ -228,7 +235,7 @@ where // Try to get module from the memory cache if let Some(module) = cache.memory_cache.load(checksum)? { - cache.stats.hits_memory_cache += 1; + cache.stats.hits_memory_cache = cache.stats.hits_memory_cache.saturating_add(1); return cache .pinned_memory_cache .store(checksum, module.module, module.size); @@ -237,7 +244,7 @@ where // Try to get module from file system cache let store = make_runtime_store(Some(cache.instance_memory_limit)); if let Some(module) = cache.fs_cache.load(checksum, &store)? { - cache.stats.hits_fs_cache += 1; + cache.stats.hits_fs_cache = cache.stats.hits_fs_cache.saturating_add(1); let module_size = loupe::size_of_val(&module); return cache .pinned_memory_cache @@ -295,20 +302,21 @@ where let mut cache = self.inner.lock().unwrap(); // Try to get module from the pinned memory cache if let Some(module) = cache.pinned_memory_cache.load(checksum)? { - cache.stats.hits_pinned_memory_cache += 1; + cache.stats.hits_pinned_memory_cache = + cache.stats.hits_pinned_memory_cache.saturating_add(1); return Ok(module); } // Get module from memory cache if let Some(module) = cache.memory_cache.load(checksum)? { - cache.stats.hits_memory_cache += 1; + cache.stats.hits_memory_cache = cache.stats.hits_memory_cache.saturating_add(1); return Ok(module.module); } // Get module from file system cache let store = make_runtime_store(Some(cache.instance_memory_limit)); if let Some(module) = cache.fs_cache.load(checksum, &store)? { - cache.stats.hits_fs_cache += 1; + cache.stats.hits_fs_cache = cache.stats.hits_fs_cache.saturating_add(1); let module_size = loupe::size_of_val(&module); cache .memory_cache @@ -322,7 +330,7 @@ where // serialization format. If you do not replay all transactions, previous calls of `save_wasm` // stored the old module format. let wasm = self.load_wasm_with_path(&cache.wasm_path, checksum)?; - cache.stats.misses += 1; + cache.stats.misses = cache.stats.misses.saturating_add(1); let module = compile(&wasm, Some(cache.instance_memory_limit), &[])?; cache.fs_cache.store(checksum, &module)?; let module_size = loupe::size_of_val(&module);