Skip to content

Commit ce57dba

Browse files
author
gabriele-0201
committed
fix: add overlay base-root and state-root consistency check
When beginning a session, ensure that the oldest overlay’s previous root matches the current state root. This prevents the creation of invalid sessions that could reuse a chain of overlays that were previously committed or or a chain that is no longer valid because another overlay chain has been committed.
1 parent 9151587 commit ce57dba

File tree

10 files changed

+130
-30
lines changed

10 files changed

+130
-30
lines changed

‎examples/commit_batch/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ impl NomtDB {
2626
// Writes do not occur immediately, instead,
2727
// they are cached and applied all at once later on
2828
let session =
29-
nomt.begin_session(SessionParams::default().witness_mode(WitnessMode::read_write()));
29+
nomt.begin_session(SessionParams::default().witness_mode(WitnessMode::read_write()))?;
3030

3131
// Here we will move the data saved under b"key1" to b"key2" and deletes it
3232
//

‎examples/read_value/src/main.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ fn main() -> Result<()> {
1616
// Instantiate a new Session object to handle read and write operations
1717
// and generate a Witness later on
1818
let session =
19-
nomt.begin_session(SessionParams::default().witness_mode(WitnessMode::read_write()));
19+
nomt.begin_session(SessionParams::default().witness_mode(WitnessMode::read_write()))?;
2020

2121
// Reading a key from the database
2222
let key_path = sha2::Sha256::digest(b"key").into();

‎fuzz/fuzz_targets/api_surface.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ fuzz_target!(|run: Run| {
1616
for call in run.calls.calls {
1717
match call {
1818
NomtCall::BeginSession { session_calls } => {
19-
let session = db.begin_session(
20-
SessionParams::default().witness_mode(WitnessMode::read_write()),
21-
);
19+
let session = db
20+
.begin_session(SessionParams::default().witness_mode(WitnessMode::read_write()))
21+
.unwrap();
2222
for session_call in session_calls {
2323
match session_call {
2424
SessionCall::TentativeRead {

‎nomt/src/lib.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -303,8 +303,9 @@ impl<T: HashAlgorithm> Nomt<T> {
303303
/// prevent writes to the database. Sessions are the main way of reading to the database,
304304
/// and permit a changeset to be committed either directly to the database or into an
305305
/// in-memory [`Overlay`].
306-
pub fn begin_session(&self, params: SessionParams) -> Session<T> {
306+
pub fn begin_session(&self, params: SessionParams) -> anyhow::Result<Session<T>> {
307307
let live_overlay = params.overlay;
308+
live_overlay.ensure_base_root(self.root())?;
308309

309310
// We must take the access guard before instantiating the rollback delta,
310311
// because it creates a read transaction and any commits or rollbacks will block
@@ -326,7 +327,7 @@ impl<T: HashAlgorithm> Nomt<T> {
326327
.parent_root()
327328
.unwrap_or_else(|| self.root().into_inner());
328329

329-
Session {
330+
Ok(Session {
330331
store,
331332
merkle_updater: self.merkle_update_pool.begin::<T>(
332333
self.page_cache.clone(),
@@ -342,7 +343,7 @@ impl<T: HashAlgorithm> Nomt<T> {
342343
access_guard,
343344
prev_root: Root(prev_root),
344345
_marker: std::marker::PhantomData,
345-
}
346+
})
346347
}
347348

348349
/// Perform a rollback of the last `n` commits.
@@ -373,7 +374,7 @@ impl<T: HashAlgorithm> Nomt<T> {
373374

374375
// We hold a write guard and don't need the session to take any other.
375376
session_params.take_global_guard = false;
376-
let sess = self.begin_session(session_params);
377+
let sess = self.begin_session(session_params)?;
377378

378379
// Convert the traceback into a series of write commands.
379380
let mut actuals = Vec::new();

‎nomt/src/overlay.rs‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ pub enum InvalidAncestors {
237237
#[derive(Clone)]
238238
pub(super) struct LiveOverlay {
239239
parent: Option<Arc<OverlayInner>>,
240+
base_root: Option<Root>,
240241
ancestor_data: Vec<Arc<Data>>,
241242
min_seqn: u64,
242243
}
@@ -250,12 +251,14 @@ impl LiveOverlay {
250251
let Some(parent) = live_ancestors.next().map(|p| p.inner.clone()) else {
251252
return Ok(LiveOverlay {
252253
parent: None,
254+
base_root: None,
253255
ancestor_data: Vec::new(),
254256
min_seqn: 0,
255257
});
256258
};
257259

258260
let mut ancestor_data = Vec::new();
261+
let mut base_root = Some(Root(parent.prev_root));
259262
for (supposed_ancestor, actual_ancestor) in live_ancestors.zip(parent.ancestor_data.iter())
260263
{
261264
let Some(actual_ancestor) = actual_ancestor.upgrade() else {
@@ -266,7 +269,8 @@ impl LiveOverlay {
266269
return Err(InvalidAncestors::NotAncestor);
267270
}
268271

269-
ancestor_data.push(actual_ancestor);
272+
base_root = Some(supposed_ancestor.prev_root());
273+
ancestor_data.push(actual_ancestor.clone());
270274
}
271275

272276
// verify that the chain is complete. The last ancestor's parent must either be `None` or
@@ -286,6 +290,7 @@ impl LiveOverlay {
286290
Ok(LiveOverlay {
287291
parent: Some(parent),
288292
ancestor_data,
293+
base_root,
289294
min_seqn,
290295
})
291296
}
@@ -418,6 +423,17 @@ impl LiveOverlay {
418423
pub(super) fn parent_root(&self) -> Option<Node> {
419424
self.parent.as_ref().map(|p| p.root)
420425
}
426+
427+
/// Ensure that the oldest overlay's previous root matches
428+
/// the specified current state root.
429+
pub fn ensure_base_root(&self, state_root: Root) -> anyhow::Result<()> {
430+
self.base_root
431+
.map_or(true, |base_root| base_root == state_root)
432+
.then(|| ())
433+
.ok_or(anyhow::anyhow!(
434+
"State root and oldest overlay prev root do not match."
435+
))
436+
}
421437
}
422438

423439
#[cfg(test)]

‎nomt/tests/common/mod.rs‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,9 @@ impl Test {
7676
o.hashtable_buckets(hashtable_buckets);
7777
o.commit_concurrency(commit_concurrency);
7878
let nomt = Nomt::open(o).unwrap();
79-
let session =
80-
nomt.begin_session(SessionParams::default().witness_mode(WitnessMode::read_write()));
79+
let session = nomt
80+
.begin_session(SessionParams::default().witness_mode(WitnessMode::read_write()))
81+
.unwrap();
8182
Self {
8283
nomt,
8384
session: Some(session),
@@ -128,7 +129,8 @@ impl Test {
128129
finished.commit(&self.nomt).unwrap();
129130
self.session = Some(
130131
self.nomt
131-
.begin_session(SessionParams::default().witness_mode(WitnessMode::read_write())),
132+
.begin_session(SessionParams::default().witness_mode(WitnessMode::read_write()))
133+
.unwrap(),
132134
);
133135
(root, witness)
134136
}
@@ -142,7 +144,8 @@ impl Test {
142144

143145
self.session = Some(
144146
self.nomt
145-
.begin_session(SessionParams::default().witness_mode(WitnessMode::read_write())),
147+
.begin_session(SessionParams::default().witness_mode(WitnessMode::read_write()))
148+
.unwrap(),
146149
);
147150

148151
(finished.into_overlay(), witness)
@@ -155,10 +158,16 @@ impl Test {
155158
overlay.commit(&self.nomt).unwrap();
156159
self.session = Some(
157160
self.nomt
158-
.begin_session(SessionParams::default().witness_mode(WitnessMode::read_write())),
161+
.begin_session(SessionParams::default().witness_mode(WitnessMode::read_write()))
162+
.unwrap(),
159163
);
160164
}
161165

166+
pub fn try_begin_session(&mut self, params: SessionParams) -> anyhow::Result<()> {
167+
self.session = Some(self.nomt.begin_session(params)?);
168+
Ok(())
169+
}
170+
162171
pub fn start_overlay_session<'a>(&mut self, ancestors: impl IntoIterator<Item = &'a Overlay>) {
163172
// force drop of live session before creating a new one.
164173
self.access.clear();
@@ -167,7 +176,7 @@ impl Test {
167176
.witness_mode(WitnessMode::read_write())
168177
.overlay(ancestors)
169178
.unwrap();
170-
self.session = Some(self.nomt.begin_session(params));
179+
self.session = Some(self.nomt.begin_session(params).unwrap());
171180
}
172181

173182
pub fn prove(&self, key: KeyPath) -> PathProof {

‎nomt/tests/overlay.rs‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,3 +252,77 @@ fn overlay_deletions() {
252252

253253
let _overlay_b = test.update().0;
254254
}
255+
256+
#[test]
257+
fn overlay_detect_alredy_committed_chain() {
258+
let mut test = Test::new("overlay_wrong_chains");
259+
260+
test.write([0; 32], Some(vec![1]));
261+
let overlay_a = test.update().0;
262+
263+
test.start_overlay_session([&overlay_a]);
264+
test.write([0; 32], Some(vec![2]));
265+
let overlay_b = test.update().0;
266+
267+
test.start_overlay_session([&overlay_b, &overlay_a]);
268+
test.write([0; 32], Some(vec![3]));
269+
let overlay_c = test.update().0;
270+
271+
let params = nomt::SessionParams::default()
272+
.witness_mode(nomt::WitnessMode::read_write())
273+
.overlay([&overlay_c, &overlay_b, &overlay_a])
274+
.unwrap();
275+
276+
test.commit_overlay(overlay_a);
277+
test.commit_overlay(overlay_b);
278+
test.commit_overlay(overlay_c);
279+
280+
assert!(test.try_begin_session(params).is_err());
281+
}
282+
283+
#[test]
284+
fn overlay_detect_parallel_non_committed_overlay_chain() {
285+
let mut test = Test::new("overlay_detect_parallel_non_committed_overlay_chain");
286+
287+
test.write([0; 32], Some(vec![1]));
288+
let overlay_a = test.update().0;
289+
290+
test.start_overlay_session([&overlay_a]);
291+
test.write([0; 32], Some(vec![2]));
292+
let overlay_b = test.update().0;
293+
294+
test.start_overlay_session([&overlay_b, &overlay_a]);
295+
test.write([0; 32], Some(vec![3]));
296+
let overlay_c = test.update().0;
297+
298+
test.write([0; 32], Some(vec![4]));
299+
let overlay_d = test.update().0;
300+
301+
test.start_overlay_session([&overlay_d]);
302+
test.write([0; 32], Some(vec![5]));
303+
let overlay_e = test.update().0;
304+
305+
test.start_overlay_session([&overlay_e, &overlay_d]);
306+
test.write([0; 32], Some(vec![6]));
307+
let overlay_f = test.update().0;
308+
309+
test.start_overlay_session([&overlay_f, &overlay_e, &overlay_d]);
310+
test.write([0; 32], Some(vec![7]));
311+
let overlay_g = test.update().0;
312+
313+
test.commit_overlay(overlay_d);
314+
test.commit_overlay(overlay_e);
315+
test.commit_overlay(overlay_f);
316+
317+
let params = nomt::SessionParams::default()
318+
.witness_mode(nomt::WitnessMode::read_write())
319+
.overlay([&overlay_c, &overlay_b, &overlay_a])
320+
.unwrap();
321+
assert!(test.try_begin_session(params).is_err());
322+
323+
let params = nomt::SessionParams::default()
324+
.witness_mode(nomt::WitnessMode::read_write())
325+
.overlay([&overlay_g])
326+
.unwrap();
327+
assert!(test.try_begin_session(params).is_ok());
328+
}

‎nomt/tests/prev_root_check.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,12 @@ fn setup_nomt(path: &str) -> Nomt<Blake3Hasher> {
2222
#[test]
2323
fn test_prev_root_commits() {
2424
let nomt = setup_nomt("prev_root_commits");
25-
let session1 = nomt.begin_session(SessionParams::default());
25+
let session1 = nomt.begin_session(SessionParams::default()).unwrap();
2626
let finished1 = session1
2727
.finish(vec![([1; 32], KeyReadWrite::Write(Some(vec![1, 2, 3])))])
2828
.unwrap();
2929

30-
let session2 = nomt.begin_session(SessionParams::default());
30+
let session2 = nomt.begin_session(SessionParams::default()).unwrap();
3131
let finished2 = session2
3232
.finish(vec![([1; 32], KeyReadWrite::Write(Some(vec![1, 2, 3])))])
3333
.unwrap();
@@ -40,13 +40,13 @@ fn test_prev_root_commits() {
4040
#[test]
4141
fn test_prev_root_overlay_invalidated() {
4242
let nomt = setup_nomt("prev_root_overlay_invalidated");
43-
let session1 = nomt.begin_session(SessionParams::default());
43+
let session1 = nomt.begin_session(SessionParams::default()).unwrap();
4444
let finished1 = session1
4545
.finish(vec![([1; 32], KeyReadWrite::Write(Some(vec![1, 2, 3])))])
4646
.unwrap();
4747
let overlay1 = finished1.into_overlay();
4848

49-
let session2 = nomt.begin_session(SessionParams::default());
49+
let session2 = nomt.begin_session(SessionParams::default()).unwrap();
5050
let finished2 = session2
5151
.finish(vec![([1; 32], KeyReadWrite::Write(Some(vec![1, 2, 3])))])
5252
.unwrap();
@@ -59,13 +59,13 @@ fn test_prev_root_overlay_invalidated() {
5959
#[test]
6060
fn test_prev_root_overlay_invalidates_session() {
6161
let nomt = setup_nomt("prev_root_overlays");
62-
let session1 = nomt.begin_session(SessionParams::default());
62+
let session1 = nomt.begin_session(SessionParams::default()).unwrap();
6363
let finished1 = session1
6464
.finish(vec![([1; 32], KeyReadWrite::Write(Some(vec![1, 2, 3])))])
6565
.unwrap();
6666
let overlay1 = finished1.into_overlay();
6767

68-
let session2 = nomt.begin_session(SessionParams::default());
68+
let session2 = nomt.begin_session(SessionParams::default()).unwrap();
6969
let finished2 = session2
7070
.finish(vec![([1; 32], KeyReadWrite::Write(Some(vec![1, 2, 3])))])
7171
.unwrap();

‎nomt/tests/rollback.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ fn test_rollback_disabled() {
4141
/* should_clean_up */ true,
4242
);
4343

44-
let session = nomt.begin_session(SessionParams::default());
44+
let session = nomt.begin_session(SessionParams::default()).unwrap();
4545
let finished = session
4646
.finish(vec![(
4747
hex!("0000000000000000000000000000000000000000000000000000000000000001"),
@@ -64,7 +64,7 @@ fn test_rollback_to_initial() {
6464
/* should_clean_up */ true,
6565
);
6666

67-
let session = nomt.begin_session(SessionParams::default());
67+
let session = nomt.begin_session(SessionParams::default()).unwrap();
6868
let finished = session
6969
.finish(vec![(
7070
hex!("0000000000000000000000000000000000000000000000000000000000000001"),
@@ -181,7 +181,7 @@ impl TestPlan {
181181
"removing keys: {}",
182182
display_keys(to_remove[commit_no].iter())
183183
);
184-
let session = nomt.begin_session(SessionParams::default());
184+
let session = nomt.begin_session(SessionParams::default()).unwrap();
185185
let mut operations = Vec::new();
186186
for (key, value) in to_insert[commit_no].iter() {
187187
operations.push((key.clone(), KeyReadWrite::Write(Some(value.clone()))));
@@ -207,7 +207,7 @@ impl TestPlan {
207207

208208
fn apply_forward(&self, nomt: &mut Nomt<Blake3Hasher>) {
209209
for commit_no in 0..self.to_insert.len() {
210-
let session = nomt.begin_session(SessionParams::default());
210+
let session = nomt.begin_session(SessionParams::default()).unwrap();
211211
let mut operations = Vec::new();
212212
for (key, value) in self.to_insert[commit_no].iter() {
213213
operations.push((key.clone(), KeyReadWrite::Write(Some(value.clone()))));
@@ -429,7 +429,7 @@ fn test_rollback_change_history() {
429429
plan.verify_restored_state(&mut nomt, 7);
430430

431431
// 3. Create new commits
432-
let session = nomt.begin_session(SessionParams::default());
432+
let session = nomt.begin_session(SessionParams::default()).unwrap();
433433
let new_key = KeyPath::from([0xAA; 32]);
434434
let new_value = vec![0xBB; 32];
435435
let finished = session
@@ -464,7 +464,7 @@ fn test_rollback_read_then_write() {
464464
);
465465

466466
// Create a new key and write a value to it
467-
let session = nomt.begin_session(SessionParams::default());
467+
let session = nomt.begin_session(SessionParams::default()).unwrap();
468468
let key = KeyPath::from([0xAA; 32]);
469469
let original_value = vec![0xBB; 32];
470470
let finished = session
@@ -479,7 +479,7 @@ fn test_rollback_read_then_write() {
479479
//
480480
// The expected behavior is that the value from the ReadThenWrite operation takes precedence
481481
// over the original value.
482-
let session = nomt.begin_session(SessionParams::default());
482+
let session = nomt.begin_session(SessionParams::default()).unwrap();
483483
assert_eq!(session.read(key).unwrap(), Some(original_value.clone()));
484484
let new_value = vec![0xCC; 32];
485485
let finished = session

‎torture/src/agent.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ impl Agent {
318318
let nomt = self.nomt.as_ref().unwrap();
319319
assert!(self.session.is_none());
320320
self.session
321-
.replace(nomt.begin_session(SessionParams::default()));
321+
.replace(nomt.begin_session(SessionParams::default()).unwrap());
322322
}
323323

324324
/// Read the specified keys from an already opened session, it requires

0 commit comments

Comments
 (0)