diff --git a/crates/precompiles-macros/src/storable.rs b/crates/precompiles-macros/src/storable.rs index 01494f885e..067936e15f 100644 --- a/crates/precompiles-macros/src/storable.rs +++ b/crates/precompiles-macros/src/storable.rs @@ -127,7 +127,7 @@ fn derive_struct_impl(input: &DeriveInput, data_struct: &DataStruct) -> syn::Res ctx: crate::storage::LayoutCtx ) -> crate::error::Result { use crate::storage::Storable; - debug_assert_eq!(ctx, crate::storage::LayoutCtx::FULL, "Struct types can only be loaded with LayoutCtx::FULL"); + debug_assert!(ctx.is_full(), "Struct types can only be loaded with a full-slot LayoutCtx (FULL or INIT)"); #load_impl @@ -144,7 +144,7 @@ fn derive_struct_impl(input: &DeriveInput, data_struct: &DataStruct) -> syn::Res ctx: crate::storage::LayoutCtx ) -> crate::error::Result<()> { use crate::storage::Storable; - debug_assert_eq!(ctx, crate::storage::LayoutCtx::FULL, "Struct types can only be stored with LayoutCtx::FULL"); + debug_assert!(ctx.is_full(), "Struct types can only be stored with a full-slot LayoutCtx (FULL or INIT)"); #store_impl @@ -157,7 +157,7 @@ fn derive_struct_impl(input: &DeriveInput, data_struct: &DataStruct) -> syn::Res ctx: crate::storage::LayoutCtx ) -> crate::error::Result<()> { use crate::storage::Storable; - debug_assert_eq!(ctx, crate::storage::LayoutCtx::FULL, "Struct types can only be deleted with LayoutCtx::FULL"); + debug_assert!(ctx.is_full(), "Struct types can only be deleted with a full-slot LayoutCtx (FULL or INIT)"); #delete_impl @@ -547,7 +547,11 @@ fn gen_store_impl(fields: &[(&Ident, &Type)], packing: &Ident) -> TokenStream { storage.store(base_slot + ::alloy::primitives::U256::from(offset), pending_val)?; pending_offset = None; } - <#ty as crate::storage::Storable>::store(&self.#name, storage, #slot_addr, crate::storage::LayoutCtx::FULL)?; + // Dynamic fields need INIT to skip stale-tail cleanup on virgin storage. + // Static fields ignore INIT, so we always use FULL for them. + let ctx = if <#ty as crate::storage::StorableType>::IS_DYNAMIC { ctx } else { crate::storage::LayoutCtx::FULL }; + debug_assert!(ctx.is_full(), "Struct types can only use full-slot LayoutCtx (FULL or INIT)"); + <#ty as crate::storage::Storable>::store(&self.#name, storage, #slot_addr, ctx)?; } // Store if this is the last field in the current slot group @@ -579,7 +583,11 @@ fn gen_store_impl(fields: &[(&Ident, &Type)], packing: &Ident) -> TokenStream { pending_offset = None; } } else { - <#ty as crate::storage::Storable>::store(&self.#name, storage, #slot_addr, crate::storage::LayoutCtx::FULL)?; + // Dynamic fields need INIT to skip stale-tail cleanup on virgin storage. + // Static fields ignore INIT, so we always use FULL for them. + let ctx = if <#ty as crate::storage::StorableType>::IS_DYNAMIC { ctx } else { crate::storage::LayoutCtx::FULL }; + debug_assert!(ctx.is_full(), "Struct types can only use full-slot LayoutCtx (FULL or INIT)"); + <#ty as crate::storage::Storable>::store(&self.#name, storage, #slot_addr, ctx)?; } }} } diff --git a/crates/precompiles/src/storage/types/bytes_like.rs b/crates/precompiles/src/storage/types/bytes_like.rs index 176663ff3b..6c904715ee 100644 --- a/crates/precompiles/src/storage/types/bytes_like.rs +++ b/crates/precompiles/src/storage/types/bytes_like.rs @@ -13,7 +13,7 @@ use crate::{ error::{Result, TempoPrecompileError}, - storage::{StorageOps, types::*}, + storage::{StorageCtx, StorageOps, types::*}, }; use alloy::primitives::{Address, Bytes, U256, keccak256}; use std::marker::PhantomData; @@ -116,20 +116,20 @@ impl Handler for BytesLikeHandler { impl Storable for Bytes { #[inline] fn load(storage: &S, slot: U256, ctx: LayoutCtx) -> Result { - debug_assert_eq!(ctx, LayoutCtx::FULL, "Bytes cannot be packed"); + debug_assert!(ctx.is_full(), "Bytes cannot be packed"); load_bytes_like(storage, slot, |data| Ok(Self::from(data))) } #[inline] fn store(&self, storage: &mut S, slot: U256, ctx: LayoutCtx) -> Result<()> { - debug_assert_eq!(ctx, LayoutCtx::FULL, "Bytes cannot be packed"); - store_bytes_like(self.as_ref(), storage, slot) + debug_assert!(ctx.is_full(), "Bytes cannot be packed"); + store_bytes_like(self.as_ref(), storage, slot, ctx) } /// Custom delete for bytes-like types: clears keccak256-addressed data slots for long values. #[inline] fn delete(storage: &mut S, slot: U256, ctx: LayoutCtx) -> Result<()> { - debug_assert_eq!(ctx, LayoutCtx::FULL, "Bytes cannot be packed"); + debug_assert!(ctx.is_full(), "Bytes cannot be packed"); delete_bytes_like(storage, slot) } } @@ -137,7 +137,7 @@ impl Storable for Bytes { impl Storable for String { #[inline] fn load(storage: &S, slot: U256, ctx: LayoutCtx) -> Result { - debug_assert_eq!(ctx, LayoutCtx::FULL, "String cannot be packed"); + debug_assert!(ctx.is_full(), "String cannot be packed"); load_bytes_like(storage, slot, |data| { Self::from_utf8(data).map_err(|e| { TempoPrecompileError::Fatal(format!("Invalid UTF-8 in stored string: {e}")) @@ -147,14 +147,14 @@ impl Storable for String { #[inline] fn store(&self, storage: &mut S, slot: U256, ctx: LayoutCtx) -> Result<()> { - debug_assert_eq!(ctx, LayoutCtx::FULL, "String cannot be packed"); - store_bytes_like(self.as_bytes(), storage, slot) + debug_assert!(ctx.is_full(), "String cannot be packed"); + store_bytes_like(self.as_bytes(), storage, slot, ctx) } /// Custom delete for bytes-like types: clears keccak256-addressed data slots for long values. #[inline] fn delete(storage: &mut S, slot: U256, ctx: LayoutCtx) -> Result<()> { - debug_assert_eq!(ctx, LayoutCtx::FULL, "String cannot be packed"); + debug_assert!(ctx.is_full(), "String cannot be packed"); delete_bytes_like(storage, slot) } } @@ -201,22 +201,47 @@ where } /// Generic store implementation for byte-like types (String, Bytes) using Solidity's encoding. +/// On T5+ performs tail cleanup when the prior value was long and the new one takes fewer slots. #[inline] -fn store_bytes_like(bytes: &[u8], storage: &mut S, base_slot: U256) -> Result<()> { - let length = bytes.len(); - if length <= 31 { +fn store_bytes_like( + bytes: &[u8], + storage: &mut S, + base_slot: U256, + ctx: LayoutCtx, +) -> Result<()> { + let new_len = bytes.len(); + let new_is_long = new_len > 31; + let mut data_slot: Option = None; + + // (T5+) Cleanup stale tail if necessary. + if !ctx.skip_tail_cleanup() && StorageCtx.spec().is_t5() { + let prev = storage.load(base_slot)?; + // Only applicable to long strings, as short ones always get overridden. + if is_long_string(prev) { + let prev_chunks = calc_chunks(calc_string_length(prev, true)?); + let new_chunks = if new_is_long { calc_chunks(new_len) } else { 0 }; + if prev_chunks > new_chunks { + let slot_start = calc_data_slot(base_slot); + for i in new_chunks..prev_chunks { + storage.store(slot_start + U256::from(i), U256::ZERO)?; + } + data_slot = Some(slot_start); + } + } + } + + if !new_is_long { storage.store(base_slot, encode_short_string(bytes)) } else { - storage.store(base_slot, encode_long_string_length(length))?; + storage.store(base_slot, encode_long_string_length(new_len))?; // Store data in chunks at keccak256(base_slot) + i - let slot_start = calc_data_slot(base_slot); - let chunks = calc_chunks(length); - + let slot_start = data_slot.unwrap_or_else(|| calc_data_slot(base_slot)); + let chunks = calc_chunks(new_len); for i in 0..chunks { let slot = slot_start + U256::from(i); let chunk_start = i * 32; - let chunk_end = (chunk_start + 32).min(length); + let chunk_end = (chunk_start + 32).min(new_len); let chunk = &bytes[chunk_start..chunk_end]; // Pad chunk to 32 bytes if it's the last chunk diff --git a/crates/precompiles/src/storage/types/mod.rs b/crates/precompiles/src/storage/types/mod.rs index 2994489d0d..35ffc719ab 100644 --- a/crates/precompiles/src/storage/types/mod.rs +++ b/crates/precompiles/src/storage/types/mod.rs @@ -87,7 +87,8 @@ impl Layout { /// ```rs /// enum LayoutCtx { /// Full, -/// Packed(usize) +/// Init, +/// Packed(usize), /// } /// ``` #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -97,10 +98,21 @@ pub struct LayoutCtx(usize); impl LayoutCtx { /// Load/store the entire value at a given slot. /// - /// For writes, this directly overwrites the entire slot without needing SLOAD. - /// All storable types support this context. + /// For writes, this signals that the value occupies full slot(s), and that the + /// implementation must clear potential stale tail data: + /// + /// - Static types overwrite the entire slot without needing an SLOAD. + /// - Dynamic types read the prior length (1 extra SLOAD) and zero any stale tail slots. pub const FULL: Self = Self(usize::MAX); + /// Like `Full`, but the asserts the destination is virgin (zero-filled). + /// + /// - Static types behave identically to `Full`. + /// - Dynamic types skip reading the prior length and clearing stale tail slots. + /// + /// Used by hot paths that know by construction the target is empty. + pub const INIT: Self = Self(usize::MAX - 1); + /// Load/store a packed primitive at the given byte offset within a slot. /// /// For writes, this requires a read-modify-write: SLOAD the current slot value, @@ -114,15 +126,29 @@ impl LayoutCtx { Self(offset) } - /// Get the packed offset, returns `None` for `Full` + /// Get the packed offset, returns `None` for `FULL` and `INIT` #[inline] pub const fn packed_offset(&self) -> Option { - if self.0 == usize::MAX { + if self.0 >= usize::MAX - 1 { None } else { Some(self.0) } } + + /// Returns `true` if this context signals the tail doesn't need to be cleared. + /// + /// Used by dynamic type's `Storable::store` to skip the extra SLOAD to check stale tails. + #[inline] + pub const fn skip_tail_cleanup(&self) -> bool { + self.0 == usize::MAX - 1 + } + + /// Returns true if this context is a full-slot context (`FULL` or `INIT`). + #[inline] + pub const fn is_full(&self) -> bool { + self.0 >= usize::MAX - 1 + } } /// Helper trait to access storage layout information without requiring const generic parameter. diff --git a/crates/precompiles/src/storage/types/vec.rs b/crates/precompiles/src/storage/types/vec.rs index 0c5b51ebae..bb531c9485 100644 --- a/crates/precompiles/src/storage/types/vec.rs +++ b/crates/precompiles/src/storage/types/vec.rs @@ -17,7 +17,7 @@ use std::ops::{Index, IndexMut}; use crate::{ error::{Result, TempoPrecompileError}, storage::{ - Handler, Layout, LayoutCtx, Storable, StorableType, StorageOps, + Handler, Layout, LayoutCtx, Storable, StorableType, StorageCtx, StorageOps, packing::{PackedSlot, calc_element_loc, calc_packed_slot_count}, types::{HandlerCache, Slot}, }, @@ -42,7 +42,7 @@ where T: Storable, { fn load(storage: &S, len_slot: U256, ctx: LayoutCtx) -> Result { - debug_assert_eq!(ctx, LayoutCtx::FULL, "Dynamic arrays cannot be packed"); + debug_assert!(ctx.is_full(), "Dynamic arrays cannot be packed"); // Read length from base slot let length = load_checked_len(storage, len_slot)?; @@ -61,9 +61,18 @@ where } fn store(&self, storage: &mut S, len_slot: U256, ctx: LayoutCtx) -> Result<()> { - debug_assert_eq!(ctx, LayoutCtx::FULL, "Dynamic arrays cannot be packed"); + debug_assert!(ctx.is_full(), "Dynamic arrays cannot be packed"); + let data_start = calc_data_slot(len_slot); + + // (T5+) Cleanup stale tail, if necessary. + if !ctx.skip_tail_cleanup() && StorageCtx.spec().is_t5() { + let (prev_len, new_len) = (load_checked_len(storage, len_slot)?, self.len()); + if prev_len > new_len { + clear_elements::(storage, data_start, new_len, prev_len)?; + } + } - // Write length to base slot + // Write length to base slot. storage.store(len_slot, U256::from(self.len()))?; if self.is_empty() { @@ -71,9 +80,8 @@ where } // Pack elements if necessary. Vec elements can't be split across slots. - let data_start = calc_data_slot(len_slot); if T::BYTES <= 16 { - store_packed_elements(self, storage, data_start, T::BYTES) + store_packed_elements(self, storage, data_start) } else { store_unpacked_elements(self, storage, data_start) } @@ -81,7 +89,7 @@ where /// Custom delete for Vec: clears both length slot and all data slots. fn delete(storage: &mut S, len_slot: U256, ctx: LayoutCtx) -> Result<()> { - debug_assert_eq!(ctx, LayoutCtx::FULL, "Dynamic arrays cannot be packed"); + debug_assert!(ctx.is_full(), "Dynamic arrays cannot be packed"); // Read length from base slot to determine how many slots to clear let length = load_checked_len(storage, len_slot)?; @@ -94,21 +102,7 @@ where } let data_start = calc_data_slot(len_slot); - if T::BYTES <= 16 { - // Clear packed element slots. Vec elements can't be split across slots. - let slot_count = calc_packed_slot_count(length, T::BYTES); - for slot_idx in 0..slot_count { - storage.store(data_start + U256::from(slot_idx), U256::ZERO)?; - } - } else { - // Clear unpacked element slots (multi-slot aware) - for elem_idx in 0..length { - let elem_slot = data_start + U256::from(elem_idx * T::SLOTS); - T::delete(storage, elem_slot, LayoutCtx::FULL)?; - } - } - - Ok(()) + clear_elements::(storage, data_start, 0, length) } } @@ -303,9 +297,17 @@ where return Err(TempoPrecompileError::Fatal("Vec is at max capacity".into())); } - // Write element at the end - let mut elem_slot = Self::compute_handler(self.data_slot(), self.address, length); - elem_slot.write(value)?; + // Write element at the end. The tail slot is empty by construction. + if T::BYTES <= 16 { + let mut elem_slot = Self::compute_handler(self.data_slot(), self.address, length); + elem_slot.write(value)?; + } else { + // Handlers always use `FULL` ctx. Since the slot we push to is guaranteed empty, + // call `T::store` with `INIT` to skip tail-cleanup SLOADs for dynamic types. + let elem_slot = self.data_slot() + U256::from(length * T::SLOTS); + let mut storage = Slot::::new(elem_slot, self.address); + value.store(&mut storage, elem_slot, LayoutCtx::INIT)?; + } // Increment length let mut length_slot = Slot::::new(self.len_slot, self.address); @@ -394,19 +396,54 @@ pub(crate) fn calc_data_slot(len_slot: U256) -> U256 { U256::from_be_bytes(alloy::primitives::keccak256(len_slot.to_be_bytes::<32>()).0) } +/// Clears storage occupied exclusively by `Vec` elements in the index range `[from, to)`. +/// +/// Packed elements (`T::BYTES <= 16`) share storage words, so this only zeroes whole packed +/// slots that contain no element before `from`: +/// +/// - `from = 0` clears from slot 0. This is the delete path and clears every slot that may contain +/// any element in `[0, to)`; e.g. for 16-byte elements, `[0, 1)` clears slot 0. +/// - `from > 0` starts at `calc_packed_slot_count(from, T::BYTES)`, intentionally preserving the +/// boundary slot that may also contain live elements `< from`; e.g. for 16-byte elements, +/// `[1, 2)` clears nothing because elements 0 and 1 share slot 0. Shrink callers rely on the +/// subsequent packed-slot rewrite to clear stale lanes in that boundary slot. +/// +/// Unpacked elements delegate to `T::delete` per element so nested dynamic storables are +/// recursively cleared. +fn clear_elements( + storage: &mut S, + data_start: U256, + from: usize, + to: usize, +) -> Result<()> { + if from >= to { + return Ok(()); + } + if T::BYTES <= 16 { + // Vec elements can't be split across slots. + let from_slots = calc_packed_slot_count(from, T::BYTES); + let to_slots = calc_packed_slot_count(to, T::BYTES); + for slot_idx in from_slots..to_slots { + storage.store(data_start + U256::from(slot_idx), U256::ZERO)?; + } + } else { + for elem_idx in from..to { + let elem_slot = data_start + U256::from(elem_idx * T::SLOTS); + T::delete(storage, elem_slot, LayoutCtx::FULL)?; + } + } + Ok(()) +} + /// Load packed elements from storage. /// /// Used when `T::BYTES <= 16`, allowing multiple elements per slot. -fn load_packed_elements( +fn load_packed_elements( storage: &S, data_start: U256, length: usize, byte_count: usize, -) -> Result> -where - T: Storable, - S: StorageOps, -{ +) -> Result> { debug_assert!( T::BYTES <= 16, "load_packed_elements requires T::BYTES <= 16" @@ -452,29 +489,24 @@ where /// Store packed elements to storage. /// /// Packs multiple small elements into each 32-byte slot using bit manipulation. -fn store_packed_elements( +fn store_packed_elements( elements: &[T], storage: &mut S, data_start: U256, - byte_count: usize, -) -> Result<()> -where - T: Storable, - S: StorageOps, -{ +) -> Result<()> { debug_assert!( T::BYTES <= 16, "store_packed_elements requires T::BYTES <= 16" ); - let elements_per_slot = 32 / byte_count; - let slot_count = calc_packed_slot_count(elements.len(), byte_count); + let elements_per_slot = 32 / T::BYTES; + let slot_count = calc_packed_slot_count(elements.len(), T::BYTES); for slot_idx in 0..slot_count { let slot_addr = data_start + U256::from(slot_idx); let start_elem = slot_idx * elements_per_slot; let end_elem = (start_elem + elements_per_slot).min(elements.len()); - let slot_value = build_packed_slot(&elements[start_elem..end_elem], byte_count)?; + let slot_value = build_packed_slot(&elements[start_elem..end_elem], T::BYTES)?; storage.store(slot_addr, slot_value)?; } @@ -484,10 +516,7 @@ where /// Build a packed storage slot from multiple elements. /// /// Takes a slice of elements and packs them into a single U256 word. -fn build_packed_slot(elements: &[T], byte_count: usize) -> Result -where - T: Storable, -{ +fn build_packed_slot(elements: &[T], byte_count: usize) -> Result { debug_assert!(T::BYTES <= 16, "build_packed_slot requires T::BYTES <= 16"); let mut slot_value = PackedSlot(U256::ZERO); let mut current_offset = 0; @@ -508,11 +537,11 @@ where /// /// Used when elements don't pack efficiently (32 bytes or multi-slot types). /// Each element occupies `T::SLOTS` consecutive slots. -fn load_unpacked_elements(storage: &S, data_start: U256, length: usize) -> Result> -where - T: Storable, - S: StorageOps, -{ +fn load_unpacked_elements( + storage: &S, + data_start: U256, + length: usize, +) -> Result> { let mut result = Vec::new(); for index in 0..length { // Use T::SLOTS for proper multi-slot element addressing @@ -526,11 +555,11 @@ where /// Store unpacked elements to storage. /// /// Each element uses `T::SLOTS` consecutive slots. -fn store_unpacked_elements(elements: &[T], storage: &mut S, data_start: U256) -> Result<()> -where - T: Storable, - S: StorageOps, -{ +fn store_unpacked_elements( + elements: &[T], + storage: &mut S, + data_start: U256, +) -> Result<()> { for (elem_idx, elem) in elements.iter().enumerate() { // Use T::SLOTS for proper multi-slot element addressing let elem_slot = data_start + U256::from(elem_idx * T::SLOTS); diff --git a/crates/precompiles/tests/storage_tests/mod.rs b/crates/precompiles/tests/storage_tests/mod.rs index d25dfcf342..7480c22aa2 100644 --- a/crates/precompiles/tests/storage_tests/mod.rs +++ b/crates/precompiles/tests/storage_tests/mod.rs @@ -18,6 +18,7 @@ mod sets; mod solidity; mod strings; mod structs; +mod vecs; // -- TEST HELPERS --------------------------------------------------------------------------------- @@ -71,6 +72,12 @@ pub(crate) fn test_address(byte: u8) -> Address { Address::from(bytes) } +/// Compute the i-th tail slot of a dynamic value (`String`/`Bytes`/`Vec`) whose +/// base/length slot is `base_slot`. Solidity stores tail data at `keccak256(base_slot) + i`. +pub(crate) fn dyn_tail_slot(base_slot: U256, i: u64) -> U256 { + U256::from_be_bytes(keccak256(base_slot.to_be_bytes::<32>()).0) + U256::from(i) +} + /// Helper to test store + load roundtrip pub(crate) fn test_store_load( address: &Address, diff --git a/crates/precompiles/tests/storage_tests/strings.rs b/crates/precompiles/tests/storage_tests/strings.rs index 734b49c0fe..9e02b8b554 100644 --- a/crates/precompiles/tests/storage_tests/strings.rs +++ b/crates/precompiles/tests/storage_tests/strings.rs @@ -1,6 +1,7 @@ //! String storage tests. use super::*; +use tempo_chainspec::hardfork::TempoHardfork; #[test] fn test_string() { @@ -81,3 +82,68 @@ proptest! { })?; } } + +// -- OVERWRITE-CLEANUP TESTS -------------------------------------------------------------- + +#[test] +fn test_string_overwrite_long_to_short_cleans_tail() -> error::Result<()> { + let address = Address::random(); + let base_slot = U256::ONE; + for &hardfork in &[TempoHardfork::T4, TempoHardfork::T5] { + let mut storage = HashMapStorageProvider::new_with_spec(1, hardfork); + StorageCtx::enter(&mut storage, || { + let mut handler = Slot::::new(base_slot, address); + + // 100 bytes -> ceil(100/32) = 4 tail chunks. + handler.write("x".repeat(100))?; + handler.write("hi".to_string())?; + assert_eq!(handler.read()?, "hi"); + + for i in 0..4 { + let chunk = Slot::::new(dyn_tail_slot(base_slot, i), address).read()?; + if hardfork.is_t5() { + assert_eq!(chunk, U256::ZERO, "T5: tail chunk {i} must clear"); + } else { + assert_ne!(chunk, U256::ZERO, "pre-T5: stale chunk {i} must persist"); + } + } + error::Result::Ok(()) + })?; + } + Ok(()) +} + +#[test] +fn test_string_overwrite_long_to_shorter_long_cleans_only_excess() -> error::Result<()> { + let address = Address::random(); + let base_slot = U256::ONE; + for &hardfork in &[TempoHardfork::T4, TempoHardfork::T5] { + let mut storage = HashMapStorageProvider::new_with_spec(1, hardfork); + StorageCtx::enter(&mut storage, || { + let mut handler = Slot::::new(base_slot, address); + + // 200 bytes -> 7 chunks; shrink to 64 bytes -> 2 chunks. + handler.write("a".repeat(200))?; + let new_value = "b".repeat(64); + handler.write(new_value.clone())?; + assert_eq!(handler.read()?, new_value); + + // Chunks 0..2 are overwritten with new data (non-zero) on both forks. + for i in 0..2 { + let chunk = Slot::::new(dyn_tail_slot(base_slot, i), address).read()?; + assert_ne!(chunk, U256::ZERO, "surviving chunk {i} must hold new data"); + } + // Chunks 2..7 fell off the tail. + for i in 2..7 { + let chunk = Slot::::new(dyn_tail_slot(base_slot, i), address).read()?; + if hardfork.is_t5() { + assert_eq!(chunk, U256::ZERO, "T5: stale chunk {i} must clear"); + } else { + assert_ne!(chunk, U256::ZERO, "pre-T5: stale chunk {i} must persist"); + } + } + error::Result::Ok(()) + })?; + } + Ok(()) +} diff --git a/crates/precompiles/tests/storage_tests/structs.rs b/crates/precompiles/tests/storage_tests/structs.rs index a4c1109099..562c16ee00 100644 --- a/crates/precompiles/tests/storage_tests/structs.rs +++ b/crates/precompiles/tests/storage_tests/structs.rs @@ -4,6 +4,7 @@ //! verifying that multi-slot structs are correctly handled and that deletion works. use super::*; +use tempo_chainspec::hardfork::TempoHardfork; use tempo_precompiles::storage::{Mapping, StorableType, StorageCtx}; #[test] @@ -290,3 +291,71 @@ proptest! { })?; } } + +// -- STRUCT OVERWRITE-CLEANUP TESTS --------------------------------------------------- + +/// Validator-config-shaped record: two long String fields plus static fields. +#[derive(Default, Debug, Clone, PartialEq, Eq, Storable)] +struct DynStringRecord { + pub static_a: U256, + pub inbound: String, + pub outbound: String, + pub static_b: u64, +} + +#[test] +fn test_struct_overwrite_cleans_dyn_field_tails() -> error::Result<()> { + let address = Address::random(); + let base_slot = U256::ONE; + for &hardfork in &[TempoHardfork::T4, TempoHardfork::T5] { + let mut storage = HashMapStorageProvider::new_with_spec(1, hardfork); + StorageCtx::enter(&mut storage, || { + let mut handler = DynStringRecord::handle(base_slot, LayoutCtx::FULL, address); + let inbound_slot = base_slot + U256::from(1); + let outbound_slot = base_slot + U256::from(2); + + // Initial: long strings in both dynamic fields. + handler.write(DynStringRecord { + static_a: U256::from(1), + inbound: "x".repeat(100), // 4 tail chunks + outbound: "y".repeat(80), // 3 tail chunks + static_b: 42, + })?; + + // Overwrite with shorter strings (validator-config-shaped path). + handler.write(DynStringRecord { + static_a: U256::from(2), + inbound: "1.2.3.4:30303".to_string(), // short + outbound: "5.6.7.8:30303".to_string(), // short + static_b: 84, + })?; + + // Logical reads return the new values. + let loaded = handler.read()?; + assert_eq!(loaded.inbound, "1.2.3.4:30303"); + assert_eq!(loaded.outbound, "5.6.7.8:30303"); + assert_eq!(loaded.static_a, U256::from(2)); + assert_eq!(loaded.static_b, 84); + + // T5: stale tail chunks of both String fields are zeroed. + for i in 0..4 { + let tail = Slot::::new(dyn_tail_slot(inbound_slot, i), address).read()?; + if hardfork.is_t5() { + assert_eq!(tail, U256::ZERO, "T5: inbound chunk {i} must clear"); + } else { + assert_ne!(tail, U256::ZERO, "T4: inbound chunk {i} shouldn't clear"); + } + } + for i in 0..3 { + let tail = Slot::::new(dyn_tail_slot(outbound_slot, i), address).read()?; + if hardfork.is_t5() { + assert_eq!(tail, U256::ZERO, "T5: outbound chunk {i} must clear"); + } else { + assert_ne!(tail, U256::ZERO, "T4: outbound chunk {i} shouldn't clear"); + } + } + error::Result::Ok(()) + })?; + } + Ok(()) +} diff --git a/crates/precompiles/tests/storage_tests/vecs.rs b/crates/precompiles/tests/storage_tests/vecs.rs new file mode 100644 index 0000000000..29b9d859c2 --- /dev/null +++ b/crates/precompiles/tests/storage_tests/vecs.rs @@ -0,0 +1,89 @@ +//! Vec overwrite-cleanup tests. + +use super::*; +use crate::storage::vec::VecHandler; +use tempo_chainspec::hardfork::TempoHardfork; + +#[test] +fn test_vec_overwrite_unpacked_cleans_tail() -> error::Result<()> { + let address = Address::random(); + let len_slot = U256::ONE; + for &hardfork in &[TempoHardfork::T4, TempoHardfork::T5] { + let mut storage = HashMapStorageProvider::new_with_spec(1, hardfork); + StorageCtx::enter(&mut storage, || { + let mut handler = VecHandler::::new(len_slot, address); + + // Seed 5 unpacked elements (U256, T::SLOTS = 1), then shrink to 2. + handler.write(vec![ + U256::from(11), + U256::from(22), + U256::from(33), + U256::from(44), + U256::from(55), + ])?; + handler.write(vec![U256::from(11), U256::from(22)])?; + assert_eq!(handler.read()?, vec![U256::from(11), U256::from(22)]); + + for (i, old) in [33u64, 44, 55].iter().enumerate() { + let idx = (i + 2) as u64; + let raw = Slot::::new(dyn_tail_slot(len_slot, idx), address).read()?; + if hardfork.is_t5() { + assert_eq!(raw, U256::ZERO, "T5: stale element {idx} must clear"); + } else { + assert_eq!(raw, U256::from(*old), "T4: stale elem {idx} must persist",); + } + } + error::Result::Ok(()) + })?; + } + Ok(()) +} + +#[test] +fn test_vec_overwrite_packed_cleans_tail() -> error::Result<()> { + let address = Address::random(); + let len_slot = U256::ONE; + for &hardfork in &[TempoHardfork::T4, TempoHardfork::T5] { + let mut storage = HashMapStorageProvider::new_with_spec(1, hardfork); + StorageCtx::enter(&mut storage, || { + let mut handler = VecHandler::::new(len_slot, address); + + // 9 u64 values -> ceil(9 * 8 / 32) = 3 slots; shrink to 3 -> 1 slot. + let initial: Vec = (1..=9).collect(); + handler.write(initial.clone())?; + assert_eq!(handler.read()?, initial); + + handler.write(vec![1u64, 2, 3])?; + assert_eq!(handler.read()?, vec![1u64, 2, 3]); + + // Slots 1 and 2 (which previously held elements [4..9]) fell off the tail. + for slot_idx in 1..3 { + let raw = Slot::::new(dyn_tail_slot(len_slot, slot_idx), address).read()?; + if hardfork.is_t5() { + assert_eq!(raw, U256::ZERO, "T5: stale slot {slot_idx} must clear"); + } else { + assert_ne!(raw, U256::ZERO, "T4: stale slot {slot_idx} must persist",); + } + } + error::Result::Ok(()) + })?; + } + Ok(()) +} + +#[test] +fn test_t5_vec_push_skips_cleanup() -> error::Result<()> { + let address = Address::random(); + let len_slot = U256::ONE; + let mut storage = HashMapStorageProvider::new_with_spec(1, TempoHardfork::T5); + StorageCtx::enter(&mut storage, || { + let mut handler = VecHandler::::new(len_slot, address); + handler.write(vec!["1".to_string(), "2".to_string(), "3".to_string()])?; + + StorageCtx.reset_counters(); + handler.push("4".to_string())?; + assert_eq!(StorageCtx.counter_sload(), 1, "push must only SLOAD length"); + assert_eq!(StorageCtx.counter_sstore(), 2, "push: element + length"); + Ok(()) + }) +}