allow defensive operations to take a proof (paritytech#11353)

* allow defensive operations to take a proof

* Update frame/support/src/traits/misc.rs

Co-authored-by: Oliver Tale-Yazdi <[email protected]>

* Update frame/bags-list/src/list/mod.rs

Co-authored-by: Keith Yeung <[email protected]>

* Update frame/support/src/traits/misc.rs

Co-authored-by: Keith Yeung <[email protected]>

* Update frame/support/src/traits/misc.rs

Co-authored-by: Keith Yeung <[email protected]>

* Fix build

* fix build again

* fmt

Co-authored-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Keith Yeung <[email protected]>

Author: 3 people

frame/bags-list/src/list/mod.rs

@@ -443,8 +443,8 @@ impl<T: Config<I>, I: 'static> List<T, I> {
 
 		// remove the heavier node from this list. Note that this removes the node from storage and
 		// decrements the node counter.
-		// defensive: both nodes have been checked to exist.
-		let _ = Self::remove(&heavier_id).defensive();
+		let _ =
+			Self::remove(&heavier_id).defensive_proof("both nodes have been checked to exist; qed");
 
 		// re-fetch `lighter_node` from storage since it may have been updated when `heavier_node`
 		// was removed.

frame/staking/src/slashing.rs

@@ -56,7 +56,7 @@ use crate::{
 use codec::{Decode, Encode};
 use frame_support::{
 	ensure,
-	traits::{Currency, Get, Imbalance, OnUnbalanced},
+	traits::{Currency, Defensive, Get, Imbalance, OnUnbalanced},
 };
 use scale_info::TypeInfo;
 use sp_runtime::{
@@ -600,8 +600,8 @@ pub fn do_slash<T: Config>(
 	slashed_imbalance: &mut NegativeImbalanceOf<T>,
 	slash_era: EraIndex,
 ) {
-	let controller = match <Pallet<T>>::bonded(stash) {
-		None => return, // defensive: should always exist.
+	let controller = match <Pallet<T>>::bonded(stash).defensive() {
+		None => return,
 		Some(c) => c,
 	};
 

frame/support/src/traits/misc.rs

@@ -50,6 +50,16 @@ macro_rules! defensive {
 			$error
 		);
 		debug_assert!(false, "{}: {:?}", $crate::traits::DEFENSIVE_OP_INTERNAL_ERROR, $error);
+	};
+	($error:tt, $proof:tt) => {
+		frame_support::log::error!(
+			target: "runtime",
+			"{}: {:?}: {:?}",
+			$crate::traits::DEFENSIVE_OP_PUBLIC_ERROR,
+			$error,
+			$proof,
+		);
+		debug_assert!(false, "{}: {:?}: {:?}", $crate::traits::DEFENSIVE_OP_INTERNAL_ERROR, $error, $proof);
 	}
 }
 
@@ -102,6 +112,10 @@ pub trait Defensive<T> {
 	/// }
 	/// ```
 	fn defensive(self) -> Self;
+
+	/// Same as [`Defensive::defensive`], but it takes a proof as input, and displays it if the
+	/// defensive operation has been triggered.
+	fn defensive_proof(self, proof: &'static str) -> Self;
 }
 
 /// Subset of methods similar to [`Defensive`] that can only work for a `Result`.
@@ -184,6 +198,13 @@ impl<T> Defensive<T> for Option<T> {
 			},
 		}
 	}
+
+	fn defensive_proof(self, proof: &'static str) -> Self {
+		if self.is_none() {
+			defensive!(proof);
+		}
+		self
+	}
 }
 
 impl<T, E: sp_std::fmt::Debug> Defensive<T> for Result<T, E> {
@@ -229,6 +250,16 @@ impl<T, E: sp_std::fmt::Debug> Defensive<T> for Result<T, E> {
 			},
 		}
 	}
+
+	fn defensive_proof(self, proof: &'static str) -> Self {
+		match self {
+			Ok(inner) => Ok(inner),
+			Err(e) => {
+				defensive!(e, proof);
+				Err(e)
+			},
+		}
+	}
 }
 
 impl<T, E: sp_std::fmt::Debug> DefensiveResult<T, E> for Result<T, E> {