risechain / pevm

Blazingly fast Parallel EVM
MIT License
236 stars 47 forks source link

Dynamically remove lazy addresses #375

Open hai-rise opened 1 month ago

hai-rise commented 1 month ago

A race condition may record a contract deployed in the same block as lazy: a latter transaction didn't recognize it had a code hash at first. We should dynamically remove it from the list of lazy addresses as:

byhashdong commented 3 weeks ago

@hai-rise, I would like to address this issue. Could you please provide a brief explanation before I proceed?

hai-rise commented 3 weeks ago

@byhashdong The easiest way to reproduce this is to add this:

diff --git a/crates/pevm/src/mv_memory.rs b/crates/pevm/src/mv_memory.rs
index bcb3e29..46e0481 100644
--- a/crates/pevm/src/mv_memory.rs
+++ b/crates/pevm/src/mv_memory.rs
@@ -80,6 +80,10 @@ impl MvMemory {
         }
     }

+    pub(crate) fn is_lazy(&self, address: &Address) -> bool {
+        self.lazy_addresses.lock().unwrap().contains(address)
+    }
+
     // Apply a new pair of read & write sets to the multi-version data structure.
     // Return whether a write occurred to a memory location not written to by
     // the previous incarnation of the same transaction. This determines whether
diff --git a/crates/pevm/src/vm.rs b/crates/pevm/src/vm.rs
index 745130b..d951713 100644
--- a/crates/pevm/src/vm.rs
+++ b/crates/pevm/src/vm.rs
@@ -168,6 +168,9 @@ impl<'a, S: Storage, C: PevmChain> VmDb<'a, S, C> {
             db.is_lazy = db.to_code_hash.is_none()
                 && (vm.mv_memory.data.contains_key(&from_hash)
                     || vm.mv_memory.data.contains_key(&to_hash.unwrap()));
+            if !db.is_lazy && vm.mv_memory.is_lazy(&to) {
+                println!("oh no");
+            }
         }
         Ok(db)
     }

Then run cargo test mainnet_blocks_from_disk -- --nocapture to see the race condition in action. A naive solution is to add a MvMemory::remove_lazy_address function to call when a new db.is_lazy is false. We may need something more sophisticated if that turns out to be too frictional, like making lazy_addresses a RwLock so at least the read check is not locked most of the time or moving the check to a colder path (construction of VmDb for all non-create transactions is a very hot path).

byhashdong commented 3 weeks ago

@hai-rise Thank you for the explanation! I will delve further into it. 😃