Skip to content

Commit ffaf9e5

Browse files
jcnelsonjcnelson
committed
chore: address PR feedback
1 parent 344b869 commit ffaf9e5

File tree

2 files changed

+140
-192
lines changed

2 files changed

+140
-192
lines changed

‎stackslib/src/clarity_vm/database/ephemeral.rs‎

Lines changed: 139 additions & 192 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,63 @@ impl<'a> EphemeralMarfStore<'a> {
312312
self.commit_to_processed_block(&bhh).unwrap();
313313
}
314314
}
315+
316+
/// Helper function to cast a Result<Option<T>, Error> into InterpreterResult<Option<T>>
317+
fn handle_marf_result<T>(res: Result<Option<T>, Error>) -> InterpreterResult<Option<T>> {
318+
match res {
319+
Ok(result_opt) => Ok(result_opt),
320+
Err(Error::NotFoundError) => {
321+
trace!("Ephemeral MarfedKV get not found",);
322+
Ok(None)
323+
}
324+
Err(e) => Err(InterpreterError::Expect(format!(
325+
"ERROR: Unexpected MARF failure: {e:?}"
326+
))
327+
.into()),
328+
}
329+
}
330+
331+
/// Helper function to implement a generic getter over the MARF for data that could be stored
332+
/// in the ephemeral MARF, but if not, could be stored in the read-only MARF. `tx_getter`
333+
/// reads from the ephemeral MARF, and `marf_getter` reads from the read-only MARF.
334+
///
335+
/// Returns Ok(Some(V)) if the key was mapped in eiher MARF
336+
/// Returns Ok(None) if the key was not mapped in either MARF
337+
/// Returns Err(InterpreterError(..)) on failure.
338+
fn get_with_fn<Key, V, TxGetter, MarfGetter>(
339+
&mut self,
340+
key: Key,
341+
tx_getter: TxGetter,
342+
marf_getter: MarfGetter,
343+
) -> InterpreterResult<Option<V>>
344+
where
345+
TxGetter: FnOnce(
346+
&mut MarfTransaction<StacksBlockId>,
347+
&StacksBlockId,
348+
Key,
349+
) -> InterpreterResult<Option<V>>,
350+
MarfGetter: FnOnce(&mut ReadOnlyMarfStore, Key) -> InterpreterResult<Option<V>>,
351+
Key: std::fmt::Debug + Copy,
352+
{
353+
let value_opt = if let EphemeralTip::RAM(tip) = &self.open_tip {
354+
// try the ephemeral MARF first
355+
tx_getter(&mut self.ephemeral_marf, tip, key)?
356+
} else {
357+
None
358+
};
359+
360+
if let Some(value) = value_opt {
361+
// found in ephemeral MARF
362+
return Ok(Some(value));
363+
}
364+
365+
// Due to the way we implemented `.set_block_hash()`, the read-only
366+
// MARF's tip will be set to `base_tip` if the open tip was ephemeral.
367+
// Otherwise, it'll be set to the tip that was last opeend. Either way,
368+
// the correct tip has been set in `self.read_only_marf` that `.get_data_from_path()`
369+
// will work as expected.
370+
marf_getter(&mut self.read_only_marf, key)
371+
}
315372
}
316373

317374
impl ClarityBackingStore for EphemeralMarfStore<'_> {
@@ -371,58 +428,27 @@ impl ClarityBackingStore for EphemeralMarfStore<'_> {
371428
/// Returns Ok(None) if the key was not mapped to the given value at the opened chain tip.
372429
/// Returns Err(..) on all other failures.
373430
fn get_data(&mut self, key: &str) -> InterpreterResult<Option<String>> {
374-
let value_res: InterpreterResult<Option<String>> = if let EphemeralTip::RAM(tip) =
431+
trace!(
432+
"Ephemeral MarfedKV get_data: {key:?} tip={:?}",
375433
&self.open_tip
376-
{
377-
// try the ephemeral MARF first
378-
self.ephemeral_marf
379-
.get(tip, key)
380-
.or_else(|e| match e {
381-
Error::NotFoundError => {
382-
test_debug!(
383-
"Ephemeral MarfedKV get {:?} off of {:?}: not found",
384-
key,
385-
tip
386-
);
387-
Ok(None)
388-
}
389-
_ => {
390-
test_debug!(
391-
"Ephemeral MarfedKV failed to get {:?} off of {:?}: {:?}",
392-
key,
393-
tip,
394-
&e
395-
);
396-
Err(e)
397-
}
398-
})
399-
.map_err(|_| InterpreterError::Expect("ERROR: Unexpected Ephemeral MARF Failure on GET".into()))?
400-
.map(|marf_value| {
401-
let side_key = marf_value.to_hex();
402-
SqliteConnection::get(self.ephemeral_marf.sqlite_conn(), &side_key)?.ok_or_else(|| {
434+
);
435+
self.get_with_fn(
436+
key,
437+
|ephemeral_marf, tip, key| {
438+
let Some(marf_value) = Self::handle_marf_result(ephemeral_marf.get(tip, key))? else {
439+
return Ok(None)
440+
};
441+
let side_key = marf_value.to_hex();
442+
let data = SqliteConnection::get(ephemeral_marf.sqlite_conn(), &side_key)?
443+
.ok_or_else(|| {
403444
InterpreterError::Expect(format!(
404-
"ERROR: Ephemeral MARF contained value_hash not found in side storage: {}",
405-
side_key
445+
"ERROR: MARF contained value_hash not found in side storage: {side_key}",
406446
))
407-
.into()
408-
})
409-
})
410-
.transpose()
411-
} else {
412-
Ok(None)
413-
};
414-
415-
if let Some(value) = value_res? {
416-
// found in ephemeral MARF
417-
return Ok(Some(value));
418-
}
419-
420-
// Due to the way we implemented `.set_block_hash()`, the read-only
421-
// MARF's tip will be set to `base_tip` if the open tip was ephemeral.
422-
// Otherwise, it'll be set to the tip that was last opeend. Either way,
423-
// the correct tip has been set in `self.read_only_marf` that `.get_data()`
424-
// will work as expected.
425-
self.read_only_marf.get_data(key)
447+
})?;
448+
Ok(Some(data))
449+
},
450+
|read_only_marf, key| read_only_marf.get_data(key)
451+
)
426452
}
427453

428454
/// Get data from the MARF given a trie hash.
@@ -435,51 +461,31 @@ impl ClarityBackingStore for EphemeralMarfStore<'_> {
435461
hash,
436462
&self.open_tip
437463
);
438-
let value_res: InterpreterResult<Option<String>> = if let EphemeralTip::RAM(tip) =
439-
&self.open_tip
440-
{
441-
// try the ephemeral MARF first
442-
self.ephemeral_marf
443-
.get_from_hash(tip, hash)
444-
.or_else(|e| match e {
445-
Error::NotFoundError => {
446-
trace!(
447-
"Ephemeral MarfedKV get {:?} off of {:?}: not found",
448-
hash,
449-
tip
450-
);
451-
Ok(None)
452-
}
453-
_ => Err(e),
454-
})
455-
.map_err(|_| InterpreterError::Expect("ERROR: Unexpected MARF Failure on get-by-path".into()))?
456-
.map(|marf_value| {
457-
let side_key = marf_value.to_hex();
458-
trace!("Ephemeral MarfedKV get side-key for {:?}: {:?}", hash, &side_key);
459-
SqliteConnection::get(self.ephemeral_marf.sqlite_conn(), &side_key)?.ok_or_else(|| {
464+
self.get_with_fn(
465+
hash,
466+
|ephemeral_marf, tip, hash| {
467+
let Some(marf_value) =
468+
Self::handle_marf_result(ephemeral_marf.get_from_hash(tip, hash))?
469+
else {
470+
return Ok(None);
471+
};
472+
let side_key = marf_value.to_hex();
473+
trace!(
474+
"Ephemeral MarfedKV get side-key for {:?}: {:?}",
475+
hash,
476+
&side_key
477+
);
478+
let data = SqliteConnection::get(ephemeral_marf.sqlite_conn(), &side_key)?
479+
.ok_or_else(|| {
460480
InterpreterError::Expect(format!(
461-
"ERROR: Ephemeral MARF contained value_hash not found in side storage: {}",
462-
side_key
463-
))
464-
.into()
465-
})
466-
})
467-
.transpose()
468-
} else {
469-
Ok(None)
470-
};
471-
472-
if let Some(value) = value_res? {
473-
// found in ephemeral MARF
474-
return Ok(Some(value));
475-
}
476-
477-
// Due to the way we implemented `.set_block_hash()`, the read-only
478-
// MARF's tip will be set to `base_tip` if the open tip was ephemeral.
479-
// Otherwise, it'll be set to the tip that was last opeend. Either way,
480-
// the correct tip has been set in `self.read_only_marf` that `.get_data_from_path()`
481-
// will work as expected.
482-
self.read_only_marf.get_data_from_path(hash)
481+
"ERROR: Ephemeral MARF contained value_hash not found in side storage: {}",
482+
side_key
483+
))
484+
})?;
485+
Ok(Some(data))
486+
},
487+
|read_only_marf, path| read_only_marf.get_data_from_path(path),
488+
)
483489
}
484490

485491
/// Get data from the MARF as well as a Merkle proof-of-inclusion.
@@ -492,55 +498,26 @@ impl ClarityBackingStore for EphemeralMarfStore<'_> {
492498
key,
493499
&self.open_tip
494500
);
495-
let value_res: InterpreterResult<Option<(String, Vec<u8>)>> =
496-
if let EphemeralTip::RAM(tip) = &self.open_tip {
497-
// try the ephemeral MARF first
498-
self.ephemeral_marf
499-
.get_with_proof(tip, key)
500-
.or_else(|e| match e {
501-
Error::NotFoundError => {
502-
trace!(
503-
"Ephemeral MarfedKV get-with-proof '{}' off of {:?}: not found",
504-
key,
505-
tip
506-
);
507-
Ok(None)
508-
}
509-
_ => Err(e),
510-
})
511-
.map_err(|_| {
512-
InterpreterError::Expect(
513-
"ERROR: Unexpected Ephemeral MARF Failure on get-with-proof".into(),
514-
)
515-
})?
516-
.map(|(marf_value, proof)| {
517-
let side_key = marf_value.to_hex();
518-
let data =
519-
SqliteConnection::get(self.ephemeral_marf.sqlite_conn(), &side_key)?
520-
.ok_or_else(|| {
521-
InterpreterError::Expect(format!(
522-
"ERROR: MARF contained value_hash not found in side storage: {}",
523-
side_key
524-
))
525-
})?;
526-
Ok((data, proof.serialize_to_vec()))
527-
})
528-
.transpose()
529-
} else {
530-
Ok(None)
531-
};
532-
533-
if let Some(value) = value_res? {
534-
// found in ephemeral MARF
535-
return Ok(Some(value));
536-
}
537-
538-
// Due to the way we implemented `.set_block_hash()`, the read-only
539-
// MARF's tip will be set to `base_tip` if the open tip was ephemeral.
540-
// Otherwise, it'll be set to the tip that was last opeend. Either way,
541-
// the correct tip has been set in `self.read_only_marf` that `.get_data_with_proof()`
542-
// will work as expected.
543-
self.read_only_marf.get_data_with_proof(key)
501+
self.get_with_fn(
502+
key,
503+
|ephemeral_marf, tip, key| {
504+
let Some((marf_value, proof)) =
505+
Self::handle_marf_result(ephemeral_marf.get_with_proof(tip, key))?
506+
else {
507+
return Ok(None);
508+
};
509+
let side_key = marf_value.to_hex();
510+
let data = SqliteConnection::get(ephemeral_marf.sqlite_conn(), &side_key)?
511+
.ok_or_else(|| {
512+
InterpreterError::Expect(format!(
513+
"ERROR: MARF contained value_hash not found in side storage: {}",
514+
side_key
515+
))
516+
})?;
517+
Ok(Some((data, proof.serialize_to_vec())))
518+
},
519+
|read_only_marf, key| read_only_marf.get_data_with_proof(key),
520+
)
544521
}
545522

546523
/// Get data and a Merkle proof-of-inclusion for it from the MARF given a trie hash.
@@ -556,56 +533,26 @@ impl ClarityBackingStore for EphemeralMarfStore<'_> {
556533
hash,
557534
&self.open_tip
558535
);
559-
let value_res: InterpreterResult<Option<(String, Vec<u8>)>> =
560-
if let EphemeralTip::RAM(tip) = &self.open_tip {
561-
self.ephemeral_marf
562-
.get_with_proof_from_hash(tip, hash)
563-
.or_else(|e| match e {
564-
Error::NotFoundError => {
565-
trace!(
566-
"Ephemeral MarfedKV get-with-proof {:?} off of {:?}: not found",
567-
hash,
568-
tip
569-
);
570-
Ok(None)
571-
}
572-
_ => Err(e),
573-
})
574-
.map_err(|_| {
575-
InterpreterError::Expect(
576-
"ERROR: Unexpected ephemeral MARF Failure on get-data-with-proof"
577-
.into(),
578-
)
579-
})?
580-
.map(|(marf_value, proof)| {
581-
let side_key = marf_value.to_hex();
582-
let data =
583-
SqliteConnection::get(self.ephemeral_marf.sqlite_conn(), &side_key)?
584-
.ok_or_else(|| {
585-
InterpreterError::Expect(format!(
586-
"ERROR: MARF contained value_hash not found in side storage: {}",
587-
side_key
588-
))
589-
})?;
590-
Ok((data, proof.serialize_to_vec()))
591-
})
592-
.transpose()
593-
} else {
594-
Ok(None)
595-
};
596-
597-
if let Some(value) = value_res? {
598-
// found in ephemeral MARF
599-
return Ok(Some(value));
600-
}
601-
602-
// Due to the way we implemented `.set_block_hash()`, the read-only
603-
// MARF's tip will be set to `base_tip` if the open tip was ephemeral.
604-
// Otherwise, it'll be set to the tip that was last opeend. Either way,
605-
// the correct tip has been set in `self.read_only_marf` that
606-
// `.get_data_with_proof_from_path()`
607-
// will work as expected.
608-
self.read_only_marf.get_data_with_proof_from_path(hash)
536+
self.get_with_fn(
537+
hash,
538+
|ephemeral_marf, tip, path| {
539+
let Some((marf_value, proof)) =
540+
Self::handle_marf_result(ephemeral_marf.get_with_proof_from_hash(tip, path))?
541+
else {
542+
return Ok(None);
543+
};
544+
let side_key = marf_value.to_hex();
545+
let data = SqliteConnection::get(ephemeral_marf.sqlite_conn(), &side_key)?
546+
.ok_or_else(|| {
547+
InterpreterError::Expect(format!(
548+
"ERROR: MARF contained value_hash not found in side storage: {}",
549+
side_key
550+
))
551+
})?;
552+
Ok(Some((data, proof.serialize_to_vec())))
553+
},
554+
|read_only_marf, path| read_only_marf.get_data_with_proof_from_path(path),
555+
)
609556
}
610557

611558
/// Get a sqlite connection to the MARF side-store.

‎stackslib/src/clarity_vm/tests/ephemeral.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,7 @@ fn replay_block(
342342
coinbase,
343343
original_block.header.pox_treatment.len(),
344344
None,
345+
Some(100),
345346
)
346347
.unwrap();
347348

0 commit comments

Comments
 (0)