fix: always include storage values for `SLOAD` and `SSTORE` in `GethTrace` #389

mattsse · 2025-12-22T23:41:26Z

should we keep all of this as it was and simply filter for changed values ad exclude warmed? I feel like this is separate from the fix

Reported changes now contain storage warming too, so I'll need to handle it somehow. I can filter out changes with had_value.is_none() which should always and only be true for storage warming. That leaves the snapshots as-is.

Would you prefer that?

I'd prefer that yes

we can still make a decision on the writer later

I reverted my changes and instead ignored the storage warming "storage changes".

-Original file line number
+Diff line change
@@ Expand Up / @@ -5,7 +5,7 @@ use super::{ @@
         },
         CallTraceArena,
     };
-    use alloc::{format, string::String};
+    use alloc::{format, string::String, vec::Vec};
     use alloy_primitives::{address, hex, map::HashMap, Address, B256, U256};
     use anstyle::{AnsiColor, Color, Style};
     use colorchoice::ColorChoice;
@@ Expand Down Expand Up / @@ -495,62 +495,41 @@ impl<W: Write> TraceWriter<W> { @@
         }
         fn write_storage_changes(&mut self, node: &CallTraceNode) -> io::Result<()> {
-            enum StorageChange {
-                Changed { value: U256, had_value: U256 },
-                Warmed { value: U256 },
-            }
-            let mut changes = HashMap::new();
+            let mut changes_map = HashMap::new();
             // For each call trace, compact the results so we do not write the intermediate storage
             // writes
             for step in &node.trace.steps {
                 if let Some(change) = &step.storage_change {
-                    changes
-                        .entry(&change.key)
-                        .and_modify(|entry| match entry {
-                            StorageChange::Changed { value, .. } => {
-                                *value = change.value;
-                            }
-                            StorageChange::Warmed { value } => {
-                                *entry =
-                                    StorageChange::Changed { value: change.value, had_value: *value };
-                            }
-                        })
-                        .or_insert_with(|| {
-                            if let Some(had_value) = change.had_value {
-                                StorageChange::Changed { value: change.value, had_value }
-                            } else {
-                                StorageChange::Warmed { value: change.value }
-                            }
-                        });
+                    let (_first, last) = changes_map.entry(&change.key).or_insert((change, change));
+                    *last = change;
                 }
             }
+            let changes = changes_map
+                .iter()
+                .filter_map(|(&&key, &(first, last))| {
+                    let value_before = first.had_value.unwrap_or_default();
+                    let value_after = last.value;
+                    if value_before == value_after {
+                        return None;
+                    }
+                    Some((key, value_before, value_after))
+                })
+                .collect::<Vec<_>>();
             if !changes.is_empty() {
                 self.write_branch()?;
                 writeln!(self.writer, " storage changes:")?;
-                for (key, change) in changes {
+                for (key, value_before, value_after) in changes {
                     self.write_pipes()?;
-                    match change {
-                        StorageChange::Changed { value, had_value } => {
-                            writeln!(
-                                self.writer,
-                                "  @ {key}: {value_before} → {value_after}",
-                                key = num_or_hex(*key),
-                                value_before = num_or_hex(had_value),
-                                value_after = num_or_hex(value),
-                            )?;
-                        }
-                        StorageChange::Warmed { value } => {
-                            writeln!(
-                                self.writer,
-                                "  @ {key}: <warmed> {value_after}",
-                                key = num_or_hex(*key),
-                                value_after = num_or_hex(value),
-                            )?;
-                        }
-                    }
+                    writeln!(
+                        self.writer,
+                        "  @ {key}: {value_before} → {value_after}",
+                        key = num_or_hex(key),
+                        value_before = num_or_hex(value_before),
+                        value_after = num_or_hex(value_after),
+                    )?;
                 }
             }
@@ Expand Down @@

-Original file line number
+Diff line change
@@ -1,4 +1,2 @@
       [2348] Counter::number()
-        ├─  storage changes:
-        │   @ 0: <warmed> 0
         └─ ← [Return] 0

-Original file line number
+Diff line change
@@ -1,4 +1,2 @@
       [2348] 0xBd770416a3345F91E4B34576cb804a576fa48EB1::8381f58a()
-        ├─  storage changes:
-        │   @ 0: <warmed> 0
         └─ ← [Return] 0x0000000000000000000000000000000000000000000000000000000000000000

-Original file line number
+Diff line change
@@ -1,4 +1,2 @@
       [2348] Counter::number()
-        ├─  storage changes:
-        │   @ 0: <warmed> 0
         └─ ← [Return] 0

-Original file line number
+Diff line change
@@ -1,4 +1,2 @@
       [2348] 0xBd770416a3345F91E4B34576cb804a576fa48EB1::8381f58a()
-        ├─  storage changes:
-        │   @ 0: <warmed> 0
         └─ ← [Return] 0x0000000000000000000000000000000000000000000000000000000000000000

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: always include storage values for `SLOAD` and `SSTORE` in `GethTrace` #389

Uh oh!

Diff view

Diff view

Uh oh!

There are no files selected for viewing

mattsse Dec 22, 2025

Uh oh!

Wodann Dec 23, 2025

Uh oh!

mattsse Dec 24, 2025

Uh oh!

Wodann Dec 29, 2025

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ -1,5 +1,3 @@
       [3711] Counter::log0()
         ├─           data: 0x00000000000000000000000000000000000000000000000000000000000000480000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000c68692066726f6d206c6f67300000000000000000000000000000000000000000
-        ├─  storage changes:
-        │   @ 0: <warmed> 72
         └─ ← [Stop]

Uh oh!

fix: always include storage values for SLOAD and SSTORE in GethTrace #389

Uh oh!

fix: always include storage values for SLOAD and SSTORE in GethTrace #389

Uh oh!

Uh oh!

Diff view

Diff view

Uh oh!

There are no files selected for viewing

mattsse Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Wodann Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

mattsse Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Wodann Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fix: always include storage values for `SLOAD` and `SSTORE` in `GethTrace` #389

fix: always include storage values for `SLOAD` and `SSTORE` in `GethTrace` #389