Skip to content

Commit

Permalink
Fix class cache serving non-declared classes in some edge cases (#1571)
Browse files Browse the repository at this point in the history
The initial assumption here was that `self.height` uniquely identifies and strictly orders the underlying state
instances. The first assumption doesn't necessarily hold, because we can pass different state instaces with the
same height. This most commonly happens with call/estimate/simulate and trace flows. Trace flow calls the VM
for block number N with the state at the beginning of the block, while call/estimate/simulate flows call the VM
with the same block number but with the state at the end of that block. That is why, we cannot use classes from cache
if they are cached on the same height that we are executing on. Because they might be cached using a state instance that
is in the future compared to the state that we are currently executing on, even tho they have the same height.
  • Loading branch information
omerfirmak authored and wojciechos committed Dec 18, 2023
1 parent de2fa17 commit e295525
Showing 1 changed file with 11 additions and 1 deletion.
12 changes: 11 additions & 1 deletion vm/rust/src/juno_state_reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,17 @@ impl StateReader for JunoStateReader {
if let Some(cached_class) = CLASS_CACHE.lock().unwrap().cache_get(class_hash) {
// skip the cache if it comes from a height higher than ours. Class might be undefined on the height
// that we are reading from right now.
if cached_class.cached_on_height <= self.height {
//
// About the changes in the second attempt at making class cache behave as expected;
//
// The initial assumption here was that `self.height` uniquely identifies and strictly orders the underlying state
// instances. The first assumption doesn't necessarily hold, because we can pass different state instaces with the
// same height. This most commonly happens with call/estimate/simulate and trace flows. Trace flow calls the VM
// for block number N with the state at the beginning of the block, while call/estimate/simulate flows call the VM
// with the same block number but with the state at the end of that block. That is why, we cannot use classes from cache
// if they are cached on the same height that we are executing on. Because they might be cached using a state instance that
// is in the future compared to the state that we are currently executing on, even tho they have the same height.
if cached_class.cached_on_height < self.height {
return Ok(cached_class.definition.clone());
}
}
Expand Down

0 comments on commit e295525

Please sign in to comment.