Skip to content

Conversation

@varun-doshi
Copy link
Contributor

Fixes #1864

@varun-doshi
Copy link
Contributor Author

@PhilippGackstatter there are some more changes to be made but I wanted get your opinion on the general direction of this change before that.

Copy link
Contributor

@PhilippGackstatter PhilippGackstatter left a comment

Choose a reason for hiding this comment

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

@varun-doshi Thank you for working on this! I took a first look and the general direction is the right one 👍

Comment on lines +18 to +19
#[derive(Debug, PartialEq, Eq, Clone, Copy, PartialOrd, Ord)]
pub struct StorageMapKey(Word);
Copy link
Contributor

Choose a reason for hiding this comment

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

We should soon have the ability to add #[derive(WordWrapper)] introduced in #2108, and once that is merged we should add it.

use crate::Felt;

#[derive(Debug, PartialEq, Eq, Clone, Copy, PartialOrd, Ord)]
pub struct HashedStorageMapKey(Word);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here regarding adding WordWrapper.

}

/// Returns the leaf index of a map key.
pub fn hashed_map_key_to_leaf_index(&self) -> Felt {
Copy link
Contributor

Choose a reason for hiding this comment

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

This could look just like AssetVault::to_leaf_index:

/// Returns the leaf index of a vault key.
pub fn to_leaf_index(&self) -> LeafIndex<SMT_DEPTH> {
LeafIndex::<SMT_DEPTH>::from(self.0)
}

@mmagician
Copy link
Contributor

@varun-doshi is this still on your radar?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Introduce Word wrapper for storage map keys

3 participants