Skip to content

Conversation

@omri-ba-starkware
Copy link

@omri-ba-starkware omri-ba-starkware commented Sep 10, 2025

This change is Reviewable

Copy link
Collaborator

@MohammadNassar1 MohammadNassar1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MohammadNassar1 reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @RoeeGross)


packages/utils/src/storage/fast_iterable_map.cairo line 32 at r1 (raw file):

    _initialized_map: Map<K, bool>,
    _head: K,
    _tail: K,

Can you please explain why you need to store the tail?

Code quote:

    _head: K,
    _tail: K,

packages/utils/src/storage/fast_iterable_map.cairo line 58 at r1 (raw file):

        self._length.read()
    }
}

I think you can use as_non_mut instead

Code quote:

impl StoragePathMutableFastIterableMapImpl<
    K, V, +Drop<K>, +Drop<V>, +Store<V>, +Hash<K, HashState>,
> of FastIterableMapTrait<StoragePath<Mutable<FastIterableMap<K, V>>>> {
    type Key = K;
    fn len(self: StoragePath<Mutable<FastIterableMap<K, V>>>) -> u64 {
        self._length.read()
    }
}

packages/utils/src/storage/fast_iterable_map.cairo line 79 at r1 (raw file):

    type Key = K;
    type Value = V;
    fn read(self: StoragePath<FastIterableMap<K, V>>, key: Self::Key) -> Option<Self::Value> {

StorageMapReadAccess trait returns Value, and not Option.

Code quote:

Option<Self::Value>

packages/utils/src/storage/fast_iterable_map.cairo line 84 at r1 (raw file):

        } else {
            Option::None
        }

I think match is better here.

Code quote:

        if self._initialized_map.entry(key).read() == true {
            Option::Some(self._inner_map.entry(key).read().value())
        } else {
            Option::None
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants