feat(storage): v2.1.0 improvements (#156)

Additional improvements to `List` implementation.

## Pull Request type

<!-- Please try to limit your pull request to one type; submit multiple
pull requests if needed. -->

Please check the type of change your PR introduces:

- [ ] Bugfix
- [x] Feature
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no API changes)
- [ ] Build-related changes
- [ ] Documentation content changes
- [ ] Other (please describe):


## What is the new behavior?

* Removing the custom `StorageSize` trait. It's replaced by
`Store::size` since in v2.1.0 it doesn't require an arg anymore.

* Using Poseidon hash instead of Pedersen (LegacyHash) since it's 10x
cheaper.

## Does this introduce a breaking change?

- [x] Yes
- [ ] No

No easy migration path but I'm guessing no one's using it in prod yet.
Since it's a breaking change, I did bump the package version to `0.2.0`.

## Other information

Also shortening the `test_append_get_many` test to go over less
elements, should be about half as long while still testing the right
behavior.

src/storage/README.md

@@ -35,35 +35,12 @@ Note that unlike `get`, using this bracket notation panics when accessing an out
 
 ### Support for custom types
 
-`List` supports most of the corelib types out of the box. However if you have a custom type that you'd like to store in a `List`, you will have to implement the `StorageSize` trait. It's a simple trait, just one function `storage_size` returning the number of storage slots necessary for your type.
-
-Let's look at an example:
-
-```rust
-use alexandria::storage::StorageSize;
-
-#[derive(starknet::StorageAccess)]
-struct Dimensions {
-    width: u128,
-    height: u128,
-    depth: u128
-}
-
-impl DimensionsStorageSize of StorageSize<Dimensions> {
-    fn storage_size() -> u8 {
-        3
-    }
-}
-```
-
-To save the `Dimensions` struct into storage, we need 3 storage slots for each of its members, as returned from the `storage_size` function.
-
-Note that for obvious reasons, any type you want to store in a `List` also has to implement the `StorageAccess` trait, hence the `#[derive(starknet::StorageAccess)]
-` above.
+`List` supports most of the corelib types out of the box. If you want to store a your own custom type in a `List`, it has to implement the `Store` trait. You can have the compiler derive it for you using the `#[derive(starknet::Store)]` attribute.
 
 ### Caveats
 
 There are two idiosyncacies you should be aware of when using `List`
+
 1. The `append` operation costs 2 storage writes - one for the value itself and another one for updating the List's length
 2. Due to a compiler limitation, it is not possible to use mutating operations with a single inline statement. For example, `self.amounts.read().append(42);` will not work. You have to do it in 2 steps:
 

src/storage/Scarb.toml

@@ -1,6 +1,6 @@
 [package]
 name = "alexandria_storage"
-version = "0.1.0"
+version = "0.2.0"
 description = "Starknet storage utilities"
 homepage = "https://github.com/keep-starknet-strange/alexandria/tree/src/storage"
 

src/storage/src/list.cairo

@@ -1,7 +1,7 @@
-use hash::LegacyHash;
+use array::ArrayTrait;
 use integer::U32DivRem;
 use option::OptionTrait;
-use array::ArrayTrait;
+use poseidon::poseidon_hash_span;
 use starknet::{storage_read_syscall, storage_write_syscall, SyscallResult, SyscallResultTrait};
 use starknet::storage_access::{
     Store, StorageBaseAddress, storage_address_to_felt252, storage_address_from_base,
@@ -29,37 +29,7 @@ trait ListTrait<T> {
     fn array(self: @List<T>) -> Array<T>;
 }
 
-// when writing elements in storage, we need to know how many storage slots
-// they take up to properly calculate the offset into a storage segment
-// that's what this trait provides
-// it's very similar to the `size` function in Store trait
-// with one crutial difference - this function does not need a T element
-// to return the size of the T element!
-trait StorageSize<T> {
-    fn storage_size() -> u8;
-}
-
-// any type that can Into to felt252 has a size of 1
-impl StorageSizeFelt252<T, impl TInto: Into<T, felt252>> of StorageSize<T> {
-    fn storage_size() -> u8 {
-        1
-    }
-}
-
-// u256 needs 2 storage slots
-impl StorageSizeU256 of StorageSize<u256> {
-    fn storage_size() -> u8 {
-        2
-    }
-}
-
-impl ListImpl<
-    T,
-    impl TCopy: Copy<T>,
-    impl TDrop: Drop<T>,
-    impl TStore: Store<T>,
-    impl TStorageSize: StorageSize<T>
-> of ListTrait<T> {
+impl ListImpl<T, impl TCopy: Copy<T>, impl TDrop: Drop<T>, impl TStore: Store<T>> of ListTrait<T> {
     fn len(self: @List<T>) -> u32 {
         *self.len
     }
@@ -131,11 +101,7 @@ impl ListImpl<
 }
 
 impl AListIndexViewImpl<
-    T,
-    impl TCopy: Copy<T>,
-    impl TDrop: Drop<T>,
-    impl TStore: Store<T>,
-    impl TStorageSize: StorageSize<T>
+    T, impl TCopy: Copy<T>, impl TDrop: Drop<T>, impl TStore: Store<T>
 > of IndexView<List<T>, u32, T> {
     fn index(self: @List<T>, index: u32) -> T {
         self.get(index).expect('List index out of bounds')
@@ -181,20 +147,23 @@ fn calculate_base_and_offset_for_index(
     let max_elements = POW2_8 / storage_size.into();
     let (key, offset) = U32DivRem::div_rem(index, max_elements.try_into().unwrap());
 
-    let segment_base = storage_base_address_from_felt252(
-        LegacyHash::hash(storage_address_to_felt252(storage_address_from_base(list_base)), key)
-    );
+    // hash the base address and the key which is the segment number
+    let addr_elements = array![
+        storage_address_to_felt252(storage_address_from_base(list_base)), key.into()
+    ];
+    let segment_base = storage_base_address_from_felt252(poseidon_hash_span(addr_elements.span()));
 
     (segment_base, offset.try_into().unwrap() * storage_size)
 }
 
-impl ListStore<T, impl TStorageSize: StorageSize<T>> of Store<List<T>> {
+impl ListStore<T, impl TStore: Store<T>> of Store<List<T>> {
     fn read(address_domain: u32, base: StorageBaseAddress) -> SyscallResult<List<T>> {
         let len: u32 = Store::read(address_domain, base).unwrap_syscall();
-        let storage_size: u8 = StorageSize::<T>::storage_size();
+        let storage_size: u8 = Store::<T>::size();
         Result::Ok(List { address_domain, base, len, storage_size })
     }
 
+    #[inline(always)]
     fn write(address_domain: u32, base: StorageBaseAddress, value: List<T>) -> SyscallResult<()> {
         Store::write(address_domain, base, value.len)
     }
@@ -203,10 +172,11 @@ impl ListStore<T, impl TStorageSize: StorageSize<T>> of Store<List<T>> {
         address_domain: u32, base: StorageBaseAddress, offset: u8
     ) -> SyscallResult<List<T>> {
         let len: u32 = Store::read_at_offset(address_domain, base, offset).unwrap_syscall();
-        let storage_size: u8 = StorageSize::<T>::storage_size();
+        let storage_size: u8 = Store::<T>::size();
         Result::Ok(List { address_domain, base, len, storage_size })
     }
 
+    #[inline(always)]
     fn write_at_offset(
         address_domain: u32, base: StorageBaseAddress, offset: u8, value: List<T>
     ) -> SyscallResult<()> {

src/storage/src/tests/list_test.cairo

@@ -25,7 +25,7 @@ mod AListHolder {
 
     #[storage]
     struct Storage {
-        // to test a corelib type that has StorageAccess and
+        // to test a corelib type that has Store and
         // Into<ContractAddress, felt252>
         addrs: List<ContractAddress>,
         // to test a corelib compound struct
@@ -136,7 +136,7 @@ mod tests {
         let mock_addr = mock_addr();
 
         let mut index: u32 = 0;
-        let max: u32 = 1025;
+        let max: u32 = 513;
 
         // test appending many
         loop {