Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add StorageIndex newtype #2428

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

feat: Add StorageIndex newtype #2428

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f583e8b
replaced storage_index from u64 to proper type
varun-doshi Dec 2, 2024
c2fc124
fmt
varun-doshi Dec 2, 2024
d127455
fixed comments and lint
varun-doshi Dec 4, 2024
dd4dbf6
fix: fixed lint
varun-doshi Dec 4, 2024
20e82d9
renamed to TrieStorageIndex and moved to Storage crate
varun-doshi Dec 10, 2024
a4152a9
convert u64 index types to TrieStorageIndex
varun-doshi Dec 11, 2024
c20bfac
merge:main
varun-doshi Dec 11, 2024
596af2e
fmt
varun-doshi Dec 11, 2024
d818c5a
updated to pass tests
varun-doshi Dec 13, 2024
5b6d3ae
resolved comments
varun-doshi Dec 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions crates/merkle-tree/src/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use pathfinder_common::{
StorageValue,
};
use pathfinder_crypto::Felt;
use pathfinder_storage::connection::storage_index::TrieStorageIndex;
use pathfinder_storage::{Transaction, TrieUpdate};

use crate::merkle_node::InternalNode;
Expand Down Expand Up @@ -66,7 +67,7 @@ impl<'tx> ContractsStorageTree<'tx> {
block: Some(block),
contract,
};
let tree = MerkleTree::new(root);
let tree = MerkleTree::new(TrieStorageIndex::new(root));
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like tx.contract_root_index() is still returning a trie storage index as an u64. It should be modified to return a Result<Option<TrieStorageIndex>> like class_root_index() and storage_root_index().


Ok(Self { tree, storage })
}
Expand Down Expand Up @@ -173,7 +174,7 @@ impl<'tx> StorageCommitmentTree<'tx> {
block: Some(block),
};

let tree = MerkleTree::new(root);
let tree = MerkleTree::new(TrieStorageIndex::new(root.get()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the TrieStorageIndex conversion here? As far as I've seen tx.storage_root_index() is now returning an Option<TrieStorageIndex> now so this seems to be redundant.


Ok(Self { tree, storage })
}
Expand Down
45 changes: 32 additions & 13 deletions crates/merkle-tree/src/merkle_node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@ use bitvec::prelude::BitVec;
use bitvec::slice::BitSlice;
use pathfinder_common::hash::FeltHash;
use pathfinder_crypto::Felt;
use pathfinder_storage::connection::storage_index::TrieStorageIndex;

/// A node in a Binary Merkle-Patricia Tree graph.
#[derive(Clone, Debug, PartialEq)]
pub enum InternalNode {
/// A node that has not been fetched from storage yet.
///
/// As such, all we know is its index.
Unresolved(u64),
Unresolved(TrieStorageIndex),
/// A branch node with exactly two children.
Binary(BinaryNode),
/// Describes a path connecting two other nodes.
Expand All @@ -32,7 +33,7 @@ pub enum InternalNode {
#[derive(Clone, Debug, PartialEq)]
pub struct BinaryNode {
/// The storage index of this node (if it was loaded from storage).
pub storage_index: Option<u64>,
pub storage_index: Option<TrieStorageIndex>,
/// The height of this node in the tree.
pub height: usize,
/// [Left](Direction::Left) child.
Expand All @@ -44,7 +45,7 @@ pub struct BinaryNode {
#[derive(Clone, Debug, PartialEq)]
pub struct EdgeNode {
/// The storage index of this node (if it was loaded from storage).
pub storage_index: Option<u64>,
pub storage_index: Option<TrieStorageIndex>,
/// The starting height of this node in the tree.
pub height: usize,
/// The path this edge takes.
Expand Down Expand Up @@ -144,9 +145,11 @@ impl InternalNode {
matches!(self, InternalNode::Leaf)
}

pub fn storage_index(&self) -> Option<u64> {
pub fn storage_index(&self) -> Option<TrieStorageIndex> {
match self {
InternalNode::Unresolved(storage_index) => Some(*storage_index),
InternalNode::Unresolved(storage_index) => {
Some(TrieStorageIndex::new(storage_index.get()))
}
InternalNode::Binary(binary) => binary.storage_index,
InternalNode::Edge(edge) => edge.storage_index,
InternalNode::Leaf => None,
Expand Down Expand Up @@ -231,8 +234,12 @@ mod tests {
let uut = BinaryNode {
storage_index: None,
height: 1,
left: Rc::new(RefCell::new(InternalNode::Unresolved(1))),
right: Rc::new(RefCell::new(InternalNode::Unresolved(2))),
left: Rc::new(RefCell::new(InternalNode::Unresolved(
TrieStorageIndex::new(1),
))),
right: Rc::new(RefCell::new(InternalNode::Unresolved(
TrieStorageIndex::new(2),
))),
};

let mut zero_key = bitvec![u8, Msb0; 1; 251];
Expand All @@ -250,8 +257,12 @@ mod tests {

#[test]
fn get_child() {
let left = Rc::new(RefCell::new(InternalNode::Unresolved(1)));
let right = Rc::new(RefCell::new(InternalNode::Unresolved(2)));
let left = Rc::new(RefCell::new(InternalNode::Unresolved(
TrieStorageIndex::new(1),
)));
let right = Rc::new(RefCell::new(InternalNode::Unresolved(
TrieStorageIndex::new(2),
)));

let uut = BinaryNode {
storage_index: None,
Expand Down Expand Up @@ -319,7 +330,9 @@ mod tests {
#[test]
fn full() {
let key = felt!("0x123456789abcdef");
let child = Rc::new(RefCell::new(InternalNode::Unresolved(1)));
let child = Rc::new(RefCell::new(InternalNode::Unresolved(
TrieStorageIndex::new(1),
)));

let uut = EdgeNode {
storage_index: None,
Expand All @@ -334,7 +347,9 @@ mod tests {
#[test]
fn prefix() {
let key = felt!("0x123456789abcdef");
let child = Rc::new(RefCell::new(InternalNode::Unresolved(1)));
let child = Rc::new(RefCell::new(InternalNode::Unresolved(
TrieStorageIndex::new(1),
)));

let path = key.view_bits()[..45].to_bitvec();

Expand All @@ -351,7 +366,9 @@ mod tests {
#[test]
fn suffix() {
let key = felt!("0x123456789abcdef");
let child = Rc::new(RefCell::new(InternalNode::Unresolved(1)));
let child = Rc::new(RefCell::new(InternalNode::Unresolved(
TrieStorageIndex::new(1),
)));

let path = key.view_bits()[50..].to_bitvec();

Expand All @@ -368,7 +385,9 @@ mod tests {
#[test]
fn middle_slice() {
let key = felt!("0x123456789abcdef");
let child = Rc::new(RefCell::new(InternalNode::Unresolved(1)));
let child = Rc::new(RefCell::new(InternalNode::Unresolved(
TrieStorageIndex::new(1),
)));

let path = key.view_bits()[230..235].to_bitvec();

Expand Down
Loading