refactor: removed duplicate definitions of storage prefixes

[nimrod-starkware]

This change is 

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 41.66667% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 72.34%. Comparing base (dc7769f) to head (c94aabb).

Files	Patch %	Lines
...src/patricia_merkle_tree/filled_tree/node_serde.rs	0.00%	4 Missing ⚠️
crates/committer/src/storage/storage_trait.rs	57.14%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #54      +/-   ##
==========================================
+ Coverage   72.04%   72.34%   +0.29%     
==========================================
  Files          18       18              
  Lines         644      640       -4     
  Branches      644      640       -4     
==========================================
- Hits          464      463       -1     
+ Misses        150      147       -3     
  Partials       30       30              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nimrod-starkware]

Reviewable status: 0 of 5 files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware and @dorimedini-starkware)

---

crates/committer/src/storage/storage_trait.rs line 0 at r1 (raw file):

Previously, AvivYossef-starkware wrote…

(Reviewable was unable to map this GitHub inline comment thread to the right spot — sorry!)

Was it a mistake?

no, i renamed it (it was PatriciaNode and now it's InnerNode)

[AvivYossef-starkware]

Reviewable status: 0 of 5 files reviewed, 4 unresolved discussions (waiting on @amosStarkware, @dorimedini-starkware, and @nimrod-starkware)

---

crates/committer/src/storage/storage_trait.rs line 40 at r2 (raw file):

/// Describes a storage prefix as used in Aerospike DB.
impl StoragePrefix {
    pub(crate) fn to_bytes(&self) -> &[u8] {

In the current implementation, the byte slices are created every time the to_bytes method is called. we lose running time. I think you need to use a static field for the enum

---

crates/committer/src/storage/storage_trait.rs line 52 at r2 (raw file):

#[allow(dead_code)]
impl StorageKey {
    pub(crate) fn with_prefix(&self, prefix: StoragePrefix) -> Self {

do you use it?

[nimrod-starkware]

Reviewable status: 0 of 5 files reviewed, 4 unresolved discussions (waiting on @amosStarkware, @AvivYossef-starkware, and @dorimedini-starkware)

---

crates/committer/src/storage/storage_trait.rs line 51 at r1 (raw file):

Previously, AvivYossef-starkware wrote…

In my opinion, this logic is confusing and will run slower than using create_db_key

Done.

---

crates/committer/src/storage/storage_trait.rs line 57 at r1 (raw file):

Previously, AvivYossef-starkware wrote…

Same as comment in line 44

Done.

---

crates/committer/src/storage/storage_trait.rs line 40 at r2 (raw file):

Previously, AvivYossef-starkware wrote…

In the current implementation, the byte slices are created every time the to_bytes method is called. we lose running time. I think you need to use a static field for the enum

good point. changed the return value to &'static [u8]

---

crates/committer/src/storage/storage_trait.rs line 52 at r2 (raw file):

Previously, AvivYossef-starkware wrote…

do you use it?

yes, currently at tests only.
changed it to #[cfg(test)]

[dorimedini-starkware]

Reviewed 1 of 1 files at r3, 7 of 7 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @amosStarkware and @nimrod-starkware)

---

crates/committer/src/patricia_merkle_tree/filled_tree/node_serde.rs line 86 at r4 (raw file):

    }

    /// Returns the db key of the filled node - [prefix + b":" + suffix].

remove the implementation detail from the docstring pls

Suggestion:

/// Returns the db key of the filled node.

---

crates/committer/src/storage/storage_trait.rs line 55 at r4 (raw file):

        create_db_key(prefix, &self.0)
    }
}

is this needed? it is strange IMO, putting test-only code in a non-test module... consider moving this to a test module (and dropping the cfg(test))

Code quote:

#[cfg(test)]
impl StorageKey {
    pub(crate) fn with_prefix(&self, prefix: StoragePrefix) -> Self {
        create_db_key(prefix, &self.0)
    }
}

[nimrod-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @amosStarkware and @dorimedini-starkware)

---

crates/committer/src/storage/storage_trait.rs line 55 at r4 (raw file):

Previously, dorimedini-starkware wrote…

is this needed? it is strange IMO, putting test-only code in a non-test module... consider moving this to a test module (and dropping the cfg(test))

removed it.

[dorimedini-starkware]

Reviewed 4 of 4 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @amosStarkware)

[AvivYossef-starkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @amosStarkware)