starknet_patricia_storage: two layer storage#13990
Conversation
PR SummaryLow Risk Overview Reads consult the overlay first; missing keys are fetched from the base (batch reads only hit the base for overlay misses). The design is aimed at use cases like Unit tests cover base fall-through, overlay shadowing, overlay delete revealing base values, and mixed Reviewed by Cursor Bugbot for commit dc4126c. Bugbot is set up for automated code reviews on this repo. Configure here. |
yoavGrs
left a comment
There was a problem hiding this comment.
@yoavGrs reviewed 2 files and all commit messages, and made 4 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on ArielElp).
a discussion (no related file):
Just a reminder, what do you need this for?
crates/starknet_patricia_storage/src/two_layer_storage.rs line 25 at r1 (raw file):
pub overlay: Overlay, pub base: &'a Base, }
Suggestion:
{
overlay: Overlay,
base: &'a Base,
}crates/starknet_patricia_storage/src/two_layer_storage.rs line 69 at r1 (raw file):
} #[cfg(test)]
Move tests to a testing file.
2de219b to
d746081
Compare
ArielElp
left a comment
There was a problem hiding this comment.
@ArielElp reviewed 1 file and made 3 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on yoavGrs).
crates/starknet_patricia_storage/src/two_layer_storage.rs line 69 at r1 (raw file):
Previously, yoavGrs wrote…
Move tests to a testing file.
Done.
crates/starknet_patricia_storage/src/two_layer_storage.rs line 25 at r1 (raw file):
pub overlay: Overlay, pub base: &'a Base, }
Done.
d746081 to
6d22672
Compare
6d471bd to
3f8b451
Compare
yoavGrs
left a comment
There was a problem hiding this comment.
@yoavGrs reviewed all commit messages, made 2 comments, and resolved 2 discussions.
Reviewable status: 1 of 3 files reviewed, 4 unresolved discussions (waiting on ArielElp).
crates/starknet_patricia_storage/src/two_layer_storage.rs line 70 at r2 (raw file):
#[cfg(test)] #[path = "two_layer_storage_test.rs"]
Move it to the top of the file.
b9ca6b2 to
c7f61b7
Compare
adada4b to
260ae4f
Compare
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware reviewed 3 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on ArielElp).
crates/starknet_patricia_storage/src/two_layer_storage.rs line 18 at r4 (raw file):
/// [`ReadOnlyStorage::get_mut`] / [`ReadOnlyStorage::mget_mut`] use the same immutable overlay and /// base paths on overlay misses so the composite implements [`ReadOnlyStorage`] while holding /// `&'a Base`. Patricia paths reads do not mutate the underlying storage.
had a stroke trying to read this first sentence, can you rephrase please?
Code quote:
/// [`ReadOnlyStorage::get_mut`] / [`ReadOnlyStorage::mget_mut`] use the same immutable overlay and
/// base paths on overlay misses so the composite implements [`ReadOnlyStorage`] while holding
/// `&'a Base`. Patricia paths reads do not mutate the underlying storage.crates/starknet_patricia_storage/src/two_layer_storage_test.rs line 40 at r4 (raw file):
two.overlay.delete(&key).await.unwrap(); assert_eq!(two.get_mut(&key).await.unwrap(), Some(base_val)); }
non-blocking - just easier to understand what the test is checking like this
Suggestion:
let mut two = TwoLayerStorage::new(MapStorage::default(), &base);
let over_val = DbValue(vec![99]);
two.overlay.set(key.clone(), over_val).await.unwrap();
assert_eq!(two.get_mut(&key).await.unwrap(), Some(over_val));
two.overlay.delete(&key).await.unwrap();
assert_eq!(two.get_mut(&key).await.unwrap(), Some(base_val));
}
yoavGrs
left a comment
There was a problem hiding this comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on ArielElp).
c7f61b7 to
adc19fa
Compare
2917668 to
9815587
Compare
adc19fa to
1e02db5
Compare
ArielElp
left a comment
There was a problem hiding this comment.
@ArielElp made 1 comment and resolved 2 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on ArielElp).
crates/starknet_patricia_storage/src/two_layer_storage.rs line 18 at r4 (raw file):
Previously, dorimedini-starkware wrote…
had a stroke trying to read this first sentence, can you rephrase please?
Done
9815587 to
827da80
Compare
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware reviewed 2 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on ArielElp).
827da80 to
55283c6
Compare
55283c6 to
9d10dee
Compare
9d10dee to
dc4126c
Compare
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware reviewed 1 file and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on ArielElp).

No description provided.