diff --git a/substrate/frame/bags-list/src/list/mod.rs b/substrate/frame/bags-list/src/list/mod.rs index f93bfca1081e958c3101b3a7eb5327f2a1dae66a..2e65c3be25b24fda52dace31c52d8764130d328e 100644 --- a/substrate/frame/bags-list/src/list/mod.rs +++ b/substrate/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. diff --git a/substrate/frame/staking/src/slashing.rs b/substrate/frame/staking/src/slashing.rs index 0d55eb7fe0ddbd4853e8c00eb6c5588e9ae83b16..25954a3699b31de81f4c73322a913f2181482764 100644 --- a/substrate/frame/staking/src/slashing.rs +++ b/substrate/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, }; diff --git a/substrate/frame/support/src/traits/misc.rs b/substrate/frame/support/src/traits/misc.rs index b67eac2fbe7bc25bddec3db8458488ca6de9419a..03420f64dd55b69bcc5e856eab62bc02a39f804c 100644 --- a/substrate/frame/support/src/traits/misc.rs +++ b/substrate/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> {