Add DefensiveTruncateFrom (#12515)

* Add DefensiveTruncateFrom

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Add tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Map_err in preimage

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Map_err in beefy

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Make test dependant in debug-assertions

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: parity-processbot <>

substrate/frame/beefy/src/lib.rs

@@ -159,7 +159,8 @@ impl<T: Config> Pallet<T> {
 		}
 
 		let bounded_authorities =
-			BoundedSlice::<T::BeefyId, T::MaxAuthorities>::try_from(authorities.as_slice())?;
+			BoundedSlice::<T::BeefyId, T::MaxAuthorities>::try_from(authorities.as_slice())
+				.map_err(|_| ())?;
 
 		let id = 0;
 		<Authorities<T>>::put(bounded_authorities);

substrate/frame/preimage/src/lib.rs

@@ -342,6 +342,7 @@ impl<T: Config> Pallet<T> {
 
 	fn insert(hash: &T::Hash, preimage: Cow<[u8]>) -> Result<(), ()> {
 		BoundedSlice::<u8, ConstU32<MAX_SIZE>>::try_from(preimage.as_ref())
+			.map_err(|_| ())
 			.map(|s| PreimageFor::<T>::insert((hash, s.len() as u32), s))
 	}
 

substrate/frame/support/src/traits.rs

@@ -56,10 +56,11 @@ mod misc;
 pub use misc::{
 	defensive_prelude::{self, *},
 	Backing, ConstBool, ConstI128, ConstI16, ConstI32, ConstI64, ConstI8, ConstU128, ConstU16,
-	ConstU32, ConstU64, ConstU8, DefensiveSaturating, EnsureInherentsAreFirst, EqualPrivilegeOnly,
+	ConstU32, ConstU64, ConstU8, DefensiveSaturating, DefensiveTruncateFrom,
-	EstimateCallFee, ExecuteBlock, ExtrinsicCall, Get, GetBacking, GetDefault, HandleLifetime,
+	EnsureInherentsAreFirst, EqualPrivilegeOnly, EstimateCallFee, ExecuteBlock, ExtrinsicCall, Get,
-	IsSubType, IsType, Len, OffchainWorker, OnKilledAccount, OnNewAccount, PrivilegeCmp,
+	GetBacking, GetDefault, HandleLifetime, IsSubType, IsType, Len, OffchainWorker,
-	SameOrOther, Time, TryCollect, TryDrop, TypedGet, UnixTime, WrapperKeepOpaque, WrapperOpaque,
+	OnKilledAccount, OnNewAccount, PrivilegeCmp, SameOrOther, Time, TryCollect, TryDrop, TypedGet,
+	UnixTime, WrapperKeepOpaque, WrapperOpaque,
 };
 #[allow(deprecated)]
 pub use misc::{PreimageProvider, PreimageRecipient};

substrate/frame/support/src/traits/misc.rs

@@ -22,6 +22,7 @@ use codec::{CompactLen, Decode, DecodeLimit, Encode, EncodeLike, Input, MaxEncod
 use impl_trait_for_tuples::impl_for_tuples;
 use scale_info::{build::Fields, meta_type, Path, Type, TypeInfo, TypeParameter};
 use sp_arithmetic::traits::{CheckedAdd, CheckedMul, CheckedSub, Saturating};
+use sp_core::bounded::bounded_vec::TruncateFrom;
 #[doc(hidden)]
 pub use sp_runtime::traits::{
 	ConstBool, ConstI128, ConstI16, ConstI32, ConstI64, ConstI8, ConstU128, ConstU16, ConstU32,
@@ -369,6 +370,43 @@ impl<T: Saturating + CheckedAdd + CheckedMul + CheckedSub> DefensiveSaturating f
 	}
 }
 
+/// Construct an object by defensively truncating an input if the `TryFrom` conversion fails.
+pub trait DefensiveTruncateFrom<T> {
+	/// Use `TryFrom` first and defensively fall back to truncating otherwise.
+	///
+	/// # Example
+	///
+	/// ```
+	/// use frame_support::{BoundedVec, traits::DefensiveTruncateFrom};
+	/// use sp_runtime::traits::ConstU32;
+	///
+	/// let unbound = vec![1, 2];
+	/// let bound = BoundedVec::<u8, ConstU32<2>>::defensive_truncate_from(unbound);
+	///
+	/// assert_eq!(bound, vec![1, 2]);
+	/// ```
+	fn defensive_truncate_from(unbound: T) -> Self;
+}
+
+impl<T, U> DefensiveTruncateFrom<U> for T
+where
+	// NOTE: We use the fact that `BoundedVec` and
+	// `BoundedSlice` use `Self` as error type. We could also
+	// require a `Clone` bound and use `unbound.clone()` in the
+	// error case.
+	T: TruncateFrom<U> + TryFrom<U, Error = U>,
+{
+	fn defensive_truncate_from(unbound: U) -> Self {
+		unbound.try_into().map_or_else(
+			|err| {
+				defensive!("DefensiveTruncateFrom truncating");
+				T::truncate_from(err)
+			},
+			|bound| bound,
+		)
+	}
+}
+
 /// Anything that can have a `::len()` method.
 pub trait Len {
 	/// Return the length of data type.
@@ -950,8 +988,59 @@ impl<Hash> PreimageRecipient<Hash> for () {
 #[cfg(test)]
 mod test {
 	use super::*;
+	use sp_core::bounded::{BoundedSlice, BoundedVec};
 	use sp_std::marker::PhantomData;
 
+	#[test]
+	#[cfg(not(debug_assertions))]
+	fn defensive_truncating_from_vec_defensive_works() {
+		let unbound = vec![1u32, 2];
+		let bound = BoundedVec::<u32, ConstU32<1>>::defensive_truncate_from(unbound);
+		assert_eq!(bound, vec![1u32]);
+	}
+
+	#[test]
+	#[cfg(not(debug_assertions))]
+	fn defensive_truncating_from_slice_defensive_works() {
+		let unbound = &[1u32, 2];
+		let bound = BoundedSlice::<u32, ConstU32<1>>::defensive_truncate_from(unbound);
+		assert_eq!(bound, &[1u32][..]);
+	}
+
+	#[test]
+	#[cfg(debug_assertions)]
+	#[should_panic(
+		expected = "Defensive failure has been triggered!: \"DefensiveTruncateFrom truncating\""
+	)]
+	fn defensive_truncating_from_vec_defensive_panics() {
+		let unbound = vec![1u32, 2];
+		let _ = BoundedVec::<u32, ConstU32<1>>::defensive_truncate_from(unbound);
+	}
+
+	#[test]
+	#[cfg(debug_assertions)]
+	#[should_panic(
+		expected = "Defensive failure has been triggered!: \"DefensiveTruncateFrom truncating\""
+	)]
+	fn defensive_truncating_from_slice_defensive_panics() {
+		let unbound = &[1u32, 2];
+		let _ = BoundedSlice::<u32, ConstU32<1>>::defensive_truncate_from(unbound);
+	}
+
+	#[test]
+	fn defensive_truncate_from_vec_works() {
+		let unbound = vec![1u32, 2, 3];
+		let bound = BoundedVec::<u32, ConstU32<3>>::defensive_truncate_from(unbound.clone());
+		assert_eq!(bound, unbound);
+	}
+
+	#[test]
+	fn defensive_truncate_from_slice_works() {
+		let unbound = [1u32, 2, 3];
+		let bound = BoundedSlice::<u32, ConstU32<3>>::defensive_truncate_from(&unbound);
+		assert_eq!(bound, &unbound[..]);
+	}
+
 	#[derive(Encode, Decode)]
 	enum NestedType {
 		Nested(Box<Self>),

substrate/primitives/core/src/bounded/bounded_vec.rs

@@ -47,6 +47,12 @@ pub struct BoundedVec<T, S>(
 	#[cfg_attr(feature = "std", serde(skip_serializing))] PhantomData<S>,
 );
 
+/// Create an object through truncation.
+pub trait TruncateFrom<T> {
+	/// Create an object through truncation.
+	fn truncate_from(unbound: T) -> Self;
+}
+
 #[cfg(feature = "std")]
 impl<'de, T, S: Get<u32>> Deserialize<'de> for BoundedVec<T, S>
 where
@@ -234,12 +240,12 @@ impl<'a, T: Ord, Bound: Get<u32>> Ord for BoundedSlice<'a, T, Bound> {
 }
 
 impl<'a, T, S: Get<u32>> TryFrom<&'a [T]> for BoundedSlice<'a, T, S> {
-	type Error = ();
+	type Error = &'a [T];
 	fn try_from(t: &'a [T]) -> Result<Self, Self::Error> {
 		if t.len() <= S::get() as usize {
 			Ok(BoundedSlice(t, PhantomData))
 		} else {
-			Err(())
+			Err(t)
 		}
 	}
 }
@@ -250,12 +256,28 @@ impl<'a, T, S> From<BoundedSlice<'a, T, S>> for &'a [T] {
 	}
 }
 
+impl<'a, T, S: Get<u32>> TruncateFrom<&'a [T]> for BoundedSlice<'a, T, S> {
+	fn truncate_from(unbound: &'a [T]) -> Self {
+		BoundedSlice::<T, S>::truncate_from(unbound)
+	}
+}
+
 impl<'a, T, S> Clone for BoundedSlice<'a, T, S> {
 	fn clone(&self) -> Self {
 		BoundedSlice(self.0, PhantomData)
 	}
 }
 
+impl<'a, T, S> sp_std::fmt::Debug for BoundedSlice<'a, T, S>
+where
+	&'a [T]: sp_std::fmt::Debug,
+	S: Get<u32>,
+{
+	fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result {
+		f.debug_tuple("BoundedSlice").field(&self.0).field(&S::get()).finish()
+	}
+}
+
 // Since a reference `&T` is always `Copy`, so is `BoundedSlice<'a, T, S>`.
 impl<'a, T, S> Copy for BoundedSlice<'a, T, S> {}
 
@@ -692,6 +714,12 @@ impl<T, S: Get<u32>> TryFrom<Vec<T>> for BoundedVec<T, S> {
 	}
 }
 
+impl<T, S: Get<u32>> TruncateFrom<Vec<T>> for BoundedVec<T, S> {
+	fn truncate_from(unbound: Vec<T>) -> Self {
+		BoundedVec::<T, S>::truncate_from(unbound)
+	}
+}
+
 // It is okay to give a non-mutable reference of the inner vec to anyone.
 impl<T, S> AsRef<Vec<T>> for BoundedVec<T, S> {
 	fn as_ref(&self) -> &Vec<T> {
@@ -809,6 +837,12 @@ where
 	}
 }
 
+impl<'a, T: PartialEq, S: Get<u32>> PartialEq<&'a [T]> for BoundedSlice<'a, T, S> {
+	fn eq(&self, other: &&'a [T]) -> bool {
+		&self.0 == other
+	}
+}
+
 impl<T: PartialEq, S: Get<u32>> PartialEq<Vec<T>> for BoundedVec<T, S> {
 	fn eq(&self, other: &Vec<T>) -> bool {
 		&self.0 == other
@@ -1219,6 +1253,18 @@ pub mod test {
 		assert!(b2.is_err());
 	}
 
+	#[test]
+	fn bounded_vec_debug_works() {
+		let bound = BoundedVec::<u32, ConstU32<5>>::truncate_from(vec![1, 2, 3]);
+		assert_eq!(format!("{:?}", bound), "BoundedVec([1, 2, 3], 5)");
+	}
+
+	#[test]
+	fn bounded_slice_debug_works() {
+		let bound = BoundedSlice::<u32, ConstU32<5>>::truncate_from(&[1, 2, 3]);
+		assert_eq!(format!("{:?}", bound), "BoundedSlice([1, 2, 3], 5)");
+	}
+
 	#[test]
 	fn bounded_vec_sort_by_key_works() {
 		let mut v: BoundedVec<i32, ConstU32<5>> = bounded_vec![-5, 4, 1, -3, 2];
@@ -1226,4 +1272,27 @@ pub mod test {
 		v.sort_by_key(|k| k.abs());
 		assert_eq!(v, vec![1, 2, -3, 4, -5]);
 	}
+
+	#[test]
+	fn bounded_vec_truncate_from_works() {
+		let unbound = vec![1, 2, 3, 4, 5];
+		let bound = BoundedVec::<u32, ConstU32<3>>::truncate_from(unbound.clone());
+		assert_eq!(bound, vec![1, 2, 3]);
+	}
+
+	#[test]
+	fn bounded_slice_truncate_from_works() {
+		let unbound = [1, 2, 3, 4, 5];
+		let bound = BoundedSlice::<u32, ConstU32<3>>::truncate_from(&unbound);
+		assert_eq!(bound, &[1, 2, 3][..]);
+	}
+
+	#[test]
+	fn bounded_slice_partialeq_slice_works() {
+		let unbound = [1, 2, 3];
+		let bound = BoundedSlice::<u32, ConstU32<3>>::truncate_from(&unbound);
+
+		assert_eq!(bound, &unbound[..]);
+		assert!(bound == &unbound[..]);
+	}
 }