From ca23f55b2e00c6c01de8fc6a2824c3db9e312ff1 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Mon, 24 Jan 2022 17:18:13 +0100 Subject: [PATCH] Proposal: Defensive trait for infallible frame operations (#10626) * add a blueprint of how defensive traits could look like * add something for arithmetic as well * add some use cases in different pallets * some build fixing * Some new stuff and examples * Fix deadly bug * add more doc. * undo faulty change to assets pallet * Update frame/support/src/traits/misc.rs Co-authored-by: Oliver Tale-Yazdi * some review comments * remove draft comment * Fix ident test * Fix proxy tests as well * a few new ideas * Fix build * Fix doc Co-authored-by: Oliver Tale-Yazdi --- frame/balances/src/lib.rs | 24 +- frame/democracy/src/lib.rs | 3 +- .../src/signed.rs | 4 +- frame/election-provider-support/src/lib.rs | 2 +- frame/elections-phragmen/src/lib.rs | 8 +- frame/gilt/src/lib.rs | 11 +- frame/identity/src/benchmarking.rs | 2 +- frame/proxy/src/benchmarking.rs | 16 +- frame/staking/src/pallet/impls.rs | 10 +- frame/support/src/traits.rs | 11 +- frame/support/src/traits/misc.rs | 355 ++++++++++++++++++ 11 files changed, 402 insertions(+), 44 deletions(-) diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index f94afec069422..8a6d54b54c8dc 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -170,7 +170,7 @@ use frame_support::{ pallet_prelude::DispatchResult, traits::{ tokens::{fungible, BalanceStatus as Status, DepositConsequence, WithdrawConsequence}, - Currency, ExistenceRequirement, + Currency, DefensiveSaturating, ExistenceRequirement, ExistenceRequirement::{AllowDeath, KeepAlive}, Get, Imbalance, LockIdentifier, LockableCurrency, NamedReservableCurrency, OnUnbalanced, ReservableCurrency, SignedImbalance, StoredMap, TryDrop, WithdrawReasons, @@ -1783,7 +1783,7 @@ where account.reserved -= actual; // defensive only: this can never fail since total issuance which is at least // free+reserved fits into the same data type. - account.free = account.free.saturating_add(actual); + account.free = account.free.defensive_saturating_add(actual); actual }) { Ok(x) => x, @@ -1896,7 +1896,7 @@ where match reserves.binary_search_by_key(id, |data| data.id) { Ok(index) => { // this add can't overflow but just to be defensive. - reserves[index].amount = reserves[index].amount.saturating_add(value); + reserves[index].amount = reserves[index].amount.defensive_saturating_add(value); }, Err(index) => { reserves @@ -1929,8 +1929,8 @@ where let remain = >::unreserve(who, to_change); - // remain should always be zero but just to be defensive here - let actual = to_change.saturating_sub(remain); + // remain should always be zero but just to be defensive here. + let actual = to_change.defensive_saturating_sub(remain); // `actual <= to_change` and `to_change <= amount`; qed; reserves[index].amount -= actual; @@ -1976,8 +1976,8 @@ where let (imb, remain) = >::slash_reserved(who, to_change); - // remain should always be zero but just to be defensive here - let actual = to_change.saturating_sub(remain); + // remain should always be zero but just to be defensive here. + let actual = to_change.defensive_saturating_sub(remain); // `actual <= to_change` and `to_change <= amount`; qed; reserves[index].amount -= actual; @@ -2036,12 +2036,12 @@ where )?; // remain should always be zero but just to be defensive - // here - let actual = to_change.saturating_sub(remain); + // here. + let actual = to_change.defensive_saturating_sub(remain); // this add can't overflow but just to be defensive. reserves[index].amount = - reserves[index].amount.saturating_add(actual); + reserves[index].amount.defensive_saturating_add(actual); Ok(actual) }, @@ -2056,7 +2056,7 @@ where // remain should always be zero but just to be defensive // here - let actual = to_change.saturating_sub(remain); + let actual = to_change.defensive_saturating_sub(remain); reserves .try_insert( @@ -2079,7 +2079,7 @@ where )?; // remain should always be zero but just to be defensive here - to_change.saturating_sub(remain) + to_change.defensive_saturating_sub(remain) }; // `actual <= to_change` and `to_change <= amount`; qed; diff --git a/frame/democracy/src/lib.rs b/frame/democracy/src/lib.rs index 1c052ad2d4176..4580767d875c3 100644 --- a/frame/democracy/src/lib.rs +++ b/frame/democracy/src/lib.rs @@ -156,6 +156,7 @@ use codec::{Decode, Encode, Input}; use frame_support::{ ensure, traits::{ + defensive_prelude::*, schedule::{DispatchTime, Named as ScheduleNamed}, BalanceStatus, Currency, Get, LockIdentifier, LockableCurrency, OnUnbalanced, ReservableCurrency, WithdrawReasons, @@ -1630,7 +1631,7 @@ impl Pallet { let mut public_props = Self::public_props(); if let Some((winner_index, _)) = public_props.iter().enumerate().max_by_key( // defensive only: All current public proposals have an amount locked - |x| Self::backing_for((x.1).0).unwrap_or_else(Zero::zero), + |x| Self::backing_for((x.1).0).defensive_unwrap_or_else(Zero::zero), ) { let (prop_index, proposal, _) = public_props.swap_remove(winner_index); >::put(public_props); diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs index c2cb5cf44823e..3b314bce80ffe 100644 --- a/frame/election-provider-multi-phase/src/signed.rs +++ b/frame/election-provider-multi-phase/src/signed.rs @@ -25,7 +25,7 @@ use crate::{ use codec::{Decode, Encode, HasCompact}; use frame_support::{ storage::bounded_btree_map::BoundedBTreeMap, - traits::{Currency, Get, OnUnbalanced, ReservableCurrency}, + traits::{defensive_prelude::*, Currency, Get, OnUnbalanced, ReservableCurrency}, }; use sp_arithmetic::traits::SaturatedConversion; use sp_npos_elections::{is_score_better, ElectionScore, NposSolution}; @@ -365,7 +365,7 @@ impl Pallet { let active_voters = raw_solution.solution.voter_count() as u32; let feasibility_weight = { // defensive only: at the end of signed phase, snapshot will exits. - let desired_targets = Self::desired_targets().unwrap_or_default(); + let desired_targets = Self::desired_targets().defensive_unwrap_or_default(); T::WeightInfo::feasibility_check(voters, targets, active_voters, desired_targets) }; // the feasibility check itself has some weight diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index d10504c88cc67..889606da3428a 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -343,7 +343,7 @@ impl ElectionProvider for NoElection<(AccountId, BlockNu /// used on the implementing side of [`ElectionDataProvider`]. pub trait SortedListProvider { /// The list's error type. - type Error; + type Error: sp_std::fmt::Debug; /// An iterator over the list, which can have `take` called on it. fn iter() -> Box>; diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 7f576144d1368..2c53bb2222b76 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -102,9 +102,9 @@ use codec::{Decode, Encode}; use frame_support::{ dispatch::WithPostDispatchInfo, traits::{ - ChangeMembers, Contains, ContainsLengthBound, Currency, CurrencyToVote, Get, - InitializeMembers, LockIdentifier, LockableCurrency, OnUnbalanced, ReservableCurrency, - SortedMembers, StorageVersion, WithdrawReasons, + defensive_prelude::*, ChangeMembers, Contains, ContainsLengthBound, Currency, + CurrencyToVote, Get, InitializeMembers, LockIdentifier, LockableCurrency, OnUnbalanced, + ReservableCurrency, SortedMembers, StorageVersion, WithdrawReasons, }, weights::Weight, }; @@ -1028,7 +1028,7 @@ impl Pallet { candidates_and_deposit .iter() .find_map(|(c, d)| if c == x { Some(*d) } else { None }) - .unwrap_or_default() + .defensive_unwrap_or_default() }; // fetch deposits from the one recorded one. This will make sure that a candidate who // submitted candidacy before a change to candidacy deposit will have the correct amount diff --git a/frame/gilt/src/lib.rs b/frame/gilt/src/lib.rs index 6211b15d8c8c3..ac56dc108b2f6 100644 --- a/frame/gilt/src/lib.rs +++ b/frame/gilt/src/lib.rs @@ -79,7 +79,7 @@ pub mod pallet { pub use crate::weights::WeightInfo; use frame_support::{ pallet_prelude::*, - traits::{Currency, OnUnbalanced, ReservableCurrency}, + traits::{Currency, DefensiveSaturating, OnUnbalanced, ReservableCurrency}, }; use frame_system::pallet_prelude::*; use sp_arithmetic::{PerThing, Perquintill}; @@ -599,10 +599,12 @@ pub mod pallet { remaining -= amount; // Should never underflow since it should track the total of the // bids exactly, but we'll be defensive. - qs[queue_index].1 = qs[queue_index].1.saturating_sub(bid.amount); + qs[queue_index].1 = + qs[queue_index].1.defensive_saturating_sub(bid.amount); // Now to activate the bid... - let nongilt_issuance = total_issuance.saturating_sub(totals.frozen); + let nongilt_issuance = + total_issuance.defensive_saturating_sub(totals.frozen); let effective_issuance = totals .proportion .left_from_one() @@ -613,7 +615,8 @@ pub mod pallet { let who = bid.who; let index = totals.index; totals.frozen += bid.amount; - totals.proportion = totals.proportion.saturating_add(proportion); + totals.proportion = + totals.proportion.defensive_saturating_add(proportion); totals.index += 1; let e = Event::GiltIssued { index, expiry, who: who.clone(), amount }; diff --git a/frame/identity/src/benchmarking.rs b/frame/identity/src/benchmarking.rs index 2f52aff394b7e..2145779ecf541 100644 --- a/frame/identity/src/benchmarking.rs +++ b/frame/identity/src/benchmarking.rs @@ -75,7 +75,7 @@ fn create_sub_accounts( } // Set identity so `set_subs` does not fail. - let _ = T::Currency::make_free_balance_be(&who, BalanceOf::::max_value()); + let _ = T::Currency::make_free_balance_be(&who, BalanceOf::::max_value() / 2u32.into()); let info = create_identity_info::(1); Identity::::set_identity(who_origin.clone().into(), Box::new(info))?; diff --git a/frame/proxy/src/benchmarking.rs b/frame/proxy/src/benchmarking.rs index ceed2e6ab22dc..f3098c6ad1274 100644 --- a/frame/proxy/src/benchmarking.rs +++ b/frame/proxy/src/benchmarking.rs @@ -33,7 +33,7 @@ fn assert_last_event(generic_event: ::Event) { fn add_proxies(n: u32, maybe_who: Option) -> Result<(), &'static str> { let caller = maybe_who.unwrap_or_else(|| whitelisted_caller()); - T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value() / 2u32.into()); for i in 0..n { Proxy::::add_proxy( RawOrigin::Signed(caller.clone()).into(), @@ -51,12 +51,12 @@ fn add_announcements( maybe_real: Option, ) -> Result<(), &'static str> { let caller = maybe_who.unwrap_or_else(|| account("caller", 0, SEED)); - T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value() / 2u32.into()); let real = if let Some(real) = maybe_real { real } else { let real = account("real", 0, SEED); - T::Currency::make_free_balance_be(&real, BalanceOf::::max_value()); + T::Currency::make_free_balance_be(&real, BalanceOf::::max_value() / 2u32.into()); Proxy::::add_proxy( RawOrigin::Signed(real.clone()).into(), caller.clone(), @@ -80,7 +80,7 @@ benchmarks! { let p in 1 .. (T::MaxProxies::get() - 1).into() => add_proxies::(p, None)?; // In this case the caller is the "target" proxy let caller: T::AccountId = account("target", p - 1, SEED); - T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value() / 2u32.into()); // ... and "real" is the traditional caller. This is not a typo. let real: T::AccountId = whitelisted_caller(); let call: ::Call = frame_system::Call::::remark { remark: vec![] }.into(); @@ -95,7 +95,7 @@ benchmarks! { // In this case the caller is the "target" proxy let caller: T::AccountId = account("anonymous", 0, SEED); let delegate: T::AccountId = account("target", p - 1, SEED); - T::Currency::make_free_balance_be(&delegate, BalanceOf::::max_value()); + T::Currency::make_free_balance_be(&delegate, BalanceOf::::max_value() / 2u32.into()); // ... and "real" is the traditional caller. This is not a typo. let real: T::AccountId = whitelisted_caller(); let call: ::Call = frame_system::Call::::remark { remark: vec![] }.into(); @@ -115,7 +115,7 @@ benchmarks! { let p in 1 .. (T::MaxProxies::get() - 1).into() => add_proxies::(p, None)?; // In this case the caller is the "target" proxy let caller: T::AccountId = account("target", p - 1, SEED); - T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value() / 2u32.into()); // ... and "real" is the traditional caller. This is not a typo. let real: T::AccountId = whitelisted_caller(); let call: ::Call = frame_system::Call::::remark { remark: vec![] }.into(); @@ -136,7 +136,7 @@ benchmarks! { let p in 1 .. (T::MaxProxies::get() - 1).into() => add_proxies::(p, None)?; // In this case the caller is the "target" proxy let caller: T::AccountId = account("target", p - 1, SEED); - T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value() / 2u32.into()); // ... and "real" is the traditional caller. This is not a typo. let real: T::AccountId = whitelisted_caller(); let call: ::Call = frame_system::Call::::remark { remark: vec![] }.into(); @@ -157,7 +157,7 @@ benchmarks! { let p in 1 .. (T::MaxProxies::get() - 1).into() => add_proxies::(p, None)?; // In this case the caller is the "target" proxy let caller: T::AccountId = account("target", p - 1, SEED); - T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value() / 2u32.into()); // ... and "real" is the traditional caller. This is not a typo. let real: T::AccountId = whitelisted_caller(); add_announcements::(a, Some(caller.clone()), None)?; diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 68b5c19027a9b..a72d2774dee2a 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -24,8 +24,8 @@ use frame_election_provider_support::{ use frame_support::{ pallet_prelude::*, traits::{ - Currency, CurrencyToVote, EstimateNextNewSession, Get, Imbalance, LockableCurrency, - OnUnbalanced, UnixTime, WithdrawReasons, + Currency, CurrencyToVote, Defensive, EstimateNextNewSession, Get, Imbalance, + LockableCurrency, OnUnbalanced, UnixTime, WithdrawReasons, }, weights::{Weight, WithPostDispatchInfo}, }; @@ -775,10 +775,8 @@ impl Pallet { pub fn do_add_nominator(who: &T::AccountId, nominations: Nominations) { if !Nominators::::contains_key(who) { // maybe update sorted list. Error checking is defensive-only - this should never fail. - if T::SortedListProvider::on_insert(who.clone(), Self::weight_of(who)).is_err() { - log!(warn, "attempt to insert duplicate nominator ({:#?})", who); - debug_assert!(false, "attempt to insert duplicate nominator"); - }; + let _ = T::SortedListProvider::on_insert(who.clone(), Self::weight_of(who)) + .defensive_unwrap_or_default(); debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); } diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 85a0698759b69..1928023cfe66d 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -50,12 +50,13 @@ pub use filter::{ClearFilterGuard, FilterStack, FilterStackGuard, InstanceFilter mod misc; pub use misc::{ + defensive_prelude::{self, *}, Backing, ConstBool, ConstI128, ConstI16, ConstI32, ConstI64, ConstI8, ConstU128, ConstU16, - ConstU32, ConstU64, ConstU8, EnsureInherentsAreFirst, EqualPrivilegeOnly, EstimateCallFee, - ExecuteBlock, ExtrinsicCall, Get, GetBacking, GetDefault, HandleLifetime, IsSubType, IsType, - Len, OffchainWorker, OnKilledAccount, OnNewAccount, PreimageProvider, PreimageRecipient, - PrivilegeCmp, SameOrOther, Time, TryCollect, TryDrop, UnixTime, WrapperKeepOpaque, - WrapperOpaque, + ConstU32, ConstU64, ConstU8, DefensiveSaturating, EnsureInherentsAreFirst, EqualPrivilegeOnly, + EstimateCallFee, ExecuteBlock, ExtrinsicCall, Get, GetBacking, GetDefault, HandleLifetime, + IsSubType, IsType, Len, OffchainWorker, OnKilledAccount, OnNewAccount, PreimageProvider, + PreimageRecipient, PrivilegeCmp, SameOrOther, Time, TryCollect, TryDrop, UnixTime, + WrapperKeepOpaque, WrapperOpaque, }; mod stored_map; diff --git a/frame/support/src/traits/misc.rs b/frame/support/src/traits/misc.rs index df21624c3cea0..eaada3ea2c363 100644 --- a/frame/support/src/traits/misc.rs +++ b/frame/support/src/traits/misc.rs @@ -20,9 +20,363 @@ use crate::dispatch::Parameter; use codec::{CompactLen, Decode, DecodeAll, Encode, EncodeLike, Input, MaxEncodedLen}; use scale_info::{build::Fields, meta_type, Path, Type, TypeInfo, TypeParameter}; +use sp_arithmetic::traits::{CheckedAdd, CheckedMul, CheckedSub, Saturating}; use sp_runtime::{traits::Block as BlockT, DispatchError}; use sp_std::{cmp::Ordering, prelude::*}; +const DEFENSIVE_OP_PUBLIC_ERROR: &'static str = "a defensive failure has been triggered; please report the block number at https://github.com/paritytech/substrate/issues"; +const DEFENSIVE_OP_INTERNAL_ERROR: &'static str = "Defensive failure has been triggered!"; + +/// Prelude module for all defensive traits to be imported at once. +pub mod defensive_prelude { + pub use super::{Defensive, DefensiveOption, DefensiveResult}; +} + +/// A trait to handle errors and options when you are really sure that a condition must hold, but +/// not brave enough to `expect` on it, or a default fallback value makes more sense. +/// +/// This trait mostly focuses on methods that eventually unwrap the inner value. See +/// [`DefensiveResult`] and [`DefensiveOption`] for methods that specifically apply to the +/// respective types. +/// +/// Each function in this trait will have two side effects, aside from behaving exactly as the name +/// would suggest: +/// +/// 1. It panics on `#[debug_assertions]`, so if the infallible code is reached in any of the tests, +/// you realize. +/// 2. It will log an error using the runtime logging system. This might help you detect such bugs +/// in production as well. Note that the log message, as of now, are not super expressive. Your +/// best shot of fully diagnosing the error would be to infer the block number of which the log +/// message was emitted, then re-execute that block using `check-block` or `try-runtime` +/// subcommands in substrate client. +pub trait Defensive { + /// Exactly the same as `unwrap_or`, but it does the defensive warnings explained in the trait + /// docs. + fn defensive_unwrap_or(self, other: T) -> T; + + /// Exactly the same as `unwrap_or_else`, but it does the defensive warnings explained in the + /// trait docs. + fn defensive_unwrap_or_else T>(self, f: F) -> T; + + /// Exactly the same as `unwrap_or_default`, but it does the defensive warnings explained in the + /// trait docs. + fn defensive_unwrap_or_default(self) -> T + where + T: Default; + + /// Does not alter the inner value at all, but it will log warnings if the inner value is `None` + /// or `Err`. + /// + /// In some ways, this is like `.defensive_map(|x| x)`. + /// + /// This is useful as: + /// ```nocompile + /// if let Some(inner) = maybe_value().defensive() { + /// .. + /// } + /// ``` + fn defensive(self) -> Self; +} + +/// Subset of methods similar to [`Defensive`] that can only work for a `Result`. +pub trait DefensiveResult { + /// Defensively map the error into another return type, but you are really sure that this + /// conversion should never be needed. + fn defensive_map_err F>(self, o: O) -> Result; + + /// Defensively map and unpack the value to something else (`U`), or call the default callback + /// if `Err`, which should never happen. + fn defensive_map_or_else U, F: FnOnce(T) -> U>(self, default: D, f: F) -> U; + + /// Defensively transform this result into an option, discarding the `Err` variant if it + /// happens, which should never happen. + fn defensive_ok(self) -> Option; + + /// Exactly the same as `map`, but it prints the appropriate warnings if the value being mapped + /// is `Err`. + fn defensive_map U>(self, f: F) -> Result; +} + +/// Subset of methods similar to [`Defensive`] that can only work for a `Option`. +pub trait DefensiveOption { + /// Potentially map and unpack the value to something else (`U`), or call the default callback + /// if `None`, which should never happen. + fn defensive_map_or_else U, F: FnOnce(T) -> U>(self, default: D, f: F) -> U; + + /// Defensively transform this option to a result. + fn defensive_ok_or_else E>(self, err: F) -> Result; + + /// Exactly the same as `map`, but it prints the appropriate warnings if the value being mapped + /// is `None`. + fn defensive_map U>(self, f: F) -> Option; +} + +impl Defensive for Option { + fn defensive_unwrap_or(self, or: T) -> T { + match self { + Some(inner) => inner, + None => { + debug_assert!(false, "{}", DEFENSIVE_OP_INTERNAL_ERROR); + frame_support::log::error!( + target: "runtime", + "{}", + DEFENSIVE_OP_PUBLIC_ERROR + ); + or + }, + } + } + + fn defensive_unwrap_or_else T>(self, f: F) -> T { + match self { + Some(inner) => inner, + None => { + debug_assert!(false, "{}", DEFENSIVE_OP_INTERNAL_ERROR); + frame_support::log::error!( + target: "runtime", + "{}", + DEFENSIVE_OP_PUBLIC_ERROR + ); + f() + }, + } + } + + fn defensive_unwrap_or_default(self) -> T + where + T: Default, + { + match self { + Some(inner) => inner, + None => { + debug_assert!(false, "{}", DEFENSIVE_OP_INTERNAL_ERROR); + frame_support::log::error!( + target: "runtime", + "{}", + DEFENSIVE_OP_PUBLIC_ERROR + ); + Default::default() + }, + } + } + + fn defensive(self) -> Self { + match self { + Some(inner) => Some(inner), + None => { + debug_assert!(false, "{}", DEFENSIVE_OP_INTERNAL_ERROR); + frame_support::log::error!( + target: "runtime", + "{}", + DEFENSIVE_OP_PUBLIC_ERROR + ); + None + }, + } + } +} + +impl Defensive for Result { + fn defensive_unwrap_or(self, or: T) -> T { + match self { + Ok(inner) => inner, + Err(e) => { + debug_assert!(false, "{}: {:?}", DEFENSIVE_OP_INTERNAL_ERROR, e); + frame_support::log::error!( + target: "runtime", + "{}: {:?}", + DEFENSIVE_OP_PUBLIC_ERROR, + e + ); + or + }, + } + } + + fn defensive_unwrap_or_else T>(self, f: F) -> T { + match self { + Ok(inner) => inner, + Err(e) => { + debug_assert!(false, "{}: {:?}", DEFENSIVE_OP_INTERNAL_ERROR, e); + frame_support::log::error!( + target: "runtime", + "{}: {:?}", + DEFENSIVE_OP_PUBLIC_ERROR, + e + ); + f() + }, + } + } + + fn defensive_unwrap_or_default(self) -> T + where + T: Default, + { + match self { + Ok(inner) => inner, + Err(e) => { + debug_assert!(false, "{}: {:?}", DEFENSIVE_OP_INTERNAL_ERROR, e); + frame_support::log::error!( + target: "runtime", + "{}: {:?}", + DEFENSIVE_OP_PUBLIC_ERROR, + e + ); + Default::default() + }, + } + } + + fn defensive(self) -> Self { + match self { + Ok(inner) => Ok(inner), + Err(e) => { + debug_assert!(false, "{}: {:?}", DEFENSIVE_OP_INTERNAL_ERROR, e); + frame_support::log::error!( + target: "runtime", + "{}: {:?}", + DEFENSIVE_OP_PUBLIC_ERROR, + e + ); + Err(e) + }, + } + } +} + +impl DefensiveResult for Result { + fn defensive_map_err F>(self, o: O) -> Result { + self.map_err(|e| { + debug_assert!(false, "{}: {:?}", DEFENSIVE_OP_INTERNAL_ERROR, e); + frame_support::log::error!( + target: "runtime", + "{}: {:?}", + DEFENSIVE_OP_PUBLIC_ERROR, + e + ); + o(e) + }) + } + + fn defensive_map_or_else U, F: FnOnce(T) -> U>(self, default: D, f: F) -> U { + self.map_or_else( + |e| { + debug_assert!(false, "{}: {:?}", DEFENSIVE_OP_INTERNAL_ERROR, e); + frame_support::log::error!( + target: "runtime", + "{}: {:?}", + DEFENSIVE_OP_PUBLIC_ERROR, + e + ); + default(e) + }, + f, + ) + } + + fn defensive_ok(self) -> Option { + match self { + Ok(inner) => Some(inner), + Err(e) => { + debug_assert!(false, "{}: {:?}", DEFENSIVE_OP_INTERNAL_ERROR, e); + frame_support::log::error!( + target: "runtime", + "{}: {:?}", + DEFENSIVE_OP_PUBLIC_ERROR, + e + ); + None + }, + } + } + + fn defensive_map U>(self, f: F) -> Result { + match self { + Ok(inner) => Ok(f(inner)), + Err(e) => { + debug_assert!(false, "{}: {:?}", DEFENSIVE_OP_INTERNAL_ERROR, e); + frame_support::log::error!( + target: "runtime", + "{}: {:?}", + DEFENSIVE_OP_PUBLIC_ERROR, + e + ); + Err(e) + }, + } + } +} + +impl DefensiveOption for Option { + fn defensive_map_or_else U, F: FnOnce(T) -> U>(self, default: D, f: F) -> U { + self.map_or_else( + || { + debug_assert!(false, "{}", DEFENSIVE_OP_INTERNAL_ERROR); + frame_support::log::error!( + target: "runtime", + "{}", + DEFENSIVE_OP_PUBLIC_ERROR, + ); + default() + }, + f, + ) + } + + fn defensive_ok_or_else E>(self, err: F) -> Result { + self.ok_or_else(|| { + debug_assert!(false, "{}", DEFENSIVE_OP_INTERNAL_ERROR); + frame_support::log::error!( + target: "runtime", + "{}", + DEFENSIVE_OP_PUBLIC_ERROR, + ); + err() + }) + } + + fn defensive_map U>(self, f: F) -> Option { + match self { + Some(inner) => Some(f(inner)), + None => { + debug_assert!(false, "{}", DEFENSIVE_OP_INTERNAL_ERROR); + frame_support::log::error!( + target: "runtime", + "{}", + DEFENSIVE_OP_PUBLIC_ERROR, + ); + None + }, + } + } +} + +/// A variant of [`Defensive`] with the same rationale, for the arithmetic operations where in +/// case an infallible operation fails, it saturates. +pub trait DefensiveSaturating { + /// Add `self` and `other` defensively. + fn defensive_saturating_add(self, other: Self) -> Self; + /// Subtract `other` from `self` defensively. + fn defensive_saturating_sub(self, other: Self) -> Self; + /// Multiply `self` and `other` defensively. + fn defensive_saturating_mul(self, other: Self) -> Self; +} + +// NOTE: A bit unfortunate, since T has to be bound by all the traits needed. Could make it +// `DefensiveSaturating` to mitigate. +impl DefensiveSaturating for T { + fn defensive_saturating_add(self, other: Self) -> Self { + self.checked_add(&other).defensive_unwrap_or_else(|| self.saturating_add(other)) + } + fn defensive_saturating_sub(self, other: Self) -> Self { + self.checked_sub(&other).defensive_unwrap_or_else(|| self.saturating_sub(other)) + } + fn defensive_saturating_mul(self, other: Self) -> Self { + self.checked_mul(&other).defensive_unwrap_or_else(|| self.saturating_mul(other)) + } +} + /// Try and collect into a collection `C`. pub trait TryCollect { type Error; @@ -72,6 +426,7 @@ impl Get for GetDefault { macro_rules! impl_const_get { ($name:ident, $t:ty) => { + #[derive($crate::RuntimeDebug)] pub struct $name; impl Get<$t> for $name { fn get() -> $t {