Skip to content

Commit

Permalink
Add custom error message for StorageNoopGuard (paritytech#1727)
Browse files Browse the repository at this point in the history
Expand `StorageNoopGuard` to be able to add extra context through a
custom error message. When the guard is triggered it panics with an
error message which can be defaulted, set on construction, or set after
it has been constructed.

Turn `StorageNoopGuard` into struct with `storage_root` and
`error_message` and added `from_error_message` constructor and
`set_error_message` setter.

Also added `new()` aliased to `default()`.

Closes paritytech#375

---------

Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
  • Loading branch information
3 people authored Sep 27, 2023
1 parent b9ae4bd commit 1749b43
Showing 1 changed file with 62 additions and 8 deletions.
70 changes: 62 additions & 8 deletions substrate/frame/support/src/storage/storage_noop_guard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,24 +37,48 @@
/// });
/// ```
#[must_use]
pub struct StorageNoopGuard(sp_std::vec::Vec<u8>);
pub struct StorageNoopGuard<'a> {
storage_root: sp_std::vec::Vec<u8>,
error_message: &'a str,
}

impl Default for StorageNoopGuard {
impl<'a> Default for StorageNoopGuard<'a> {
fn default() -> Self {
Self(sp_io::storage::root(sp_runtime::StateVersion::V1))
Self {
storage_root: sp_io::storage::root(sp_runtime::StateVersion::V1),
error_message: "`StorageNoopGuard` detected an attempted storage change.",
}
}
}

impl<'a> StorageNoopGuard<'a> {
/// Alias to `default()`.
pub fn new() -> Self {
Self::default()
}

/// Creates a new [`StorageNoopGuard`] with a custom error message.
pub fn from_error_message(error_message: &'a str) -> Self {
Self { storage_root: sp_io::storage::root(sp_runtime::StateVersion::V1), error_message }
}

/// Sets a custom error message for a [`StorageNoopGuard`].
pub fn set_error_message(&mut self, error_message: &'a str) {
self.error_message = error_message;
}
}

impl Drop for StorageNoopGuard {
impl<'a> Drop for StorageNoopGuard<'a> {
fn drop(&mut self) {
// No need to double panic, eg. inside a test assertion failure.
if sp_std::thread::panicking() {
return
}
assert_eq!(
sp_io::storage::root(sp_runtime::StateVersion::V1),
self.0,
"StorageNoopGuard detected wrongful storage changes.",
self.storage_root,
"{}",
self.error_message,
);
}
}
Expand All @@ -65,7 +89,7 @@ mod tests {
use sp_io::TestExternalities;

#[test]
#[should_panic(expected = "StorageNoopGuard detected wrongful storage changes.")]
#[should_panic(expected = "`StorageNoopGuard` detected an attempted storage change.")]
fn storage_noop_guard_panics_on_changed() {
TestExternalities::default().execute_with(|| {
let _guard = StorageNoopGuard::default();
Expand All @@ -83,7 +107,7 @@ mod tests {
}

#[test]
#[should_panic(expected = "StorageNoopGuard detected wrongful storage changes.")]
#[should_panic(expected = "`StorageNoopGuard` detected an attempted storage change.")]
fn storage_noop_guard_panics_on_early_drop() {
TestExternalities::default().execute_with(|| {
let guard = StorageNoopGuard::default();
Expand Down Expand Up @@ -111,4 +135,34 @@ mod tests {
panic!("Something else");
});
}

#[test]
#[should_panic(expected = "`StorageNoopGuard` found unexpected storage changes.")]
fn storage_noop_guard_panics_created_from_error_message() {
TestExternalities::default().execute_with(|| {
let _guard = StorageNoopGuard::from_error_message(
"`StorageNoopGuard` found unexpected storage changes.",
);
frame_support::storage::unhashed::put(b"key", b"value");
});
}

#[test]
#[should_panic(expected = "`StorageNoopGuard` found unexpected storage changes.")]
fn storage_noop_guard_panics_with_set_error_message() {
TestExternalities::default().execute_with(|| {
let mut guard = StorageNoopGuard::default();
guard.set_error_message("`StorageNoopGuard` found unexpected storage changes.");
frame_support::storage::unhashed::put(b"key", b"value");
});
}

#[test]
#[should_panic(expected = "`StorageNoopGuard` detected an attempted storage change.")]
fn storage_noop_guard_panics_new_alias() {
TestExternalities::default().execute_with(|| {
let _guard = StorageNoopGuard::new();
frame_support::storage::unhashed::put(b"key", b"value");
});
}
}

0 comments on commit 1749b43

Please sign in to comment.