diff --git a/substrate/frame/beefy/src/lib.rs b/substrate/frame/beefy/src/lib.rs index fce531d3f5dd77a3e9e3a355a6e7715de12c0775..305b158124b672114ee0159f290cd5af107a2d62 100644 --- a/substrate/frame/beefy/src/lib.rs +++ b/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); diff --git a/substrate/frame/preimage/src/lib.rs b/substrate/frame/preimage/src/lib.rs index e899d3643dbbffb89d7ab9e5bd1b6d140c5c3690..6549832c11f5d78c0974e6e55394cb330bf5462f 100644 --- a/substrate/frame/preimage/src/lib.rs +++ b/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)) } diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index 302d3354dae5e1b2278fda6485b095b5a9f4ccc4..2b6c5efee10bb1bd156642916e6362f6e6643b41 100644 --- a/substrate/frame/support/src/traits.rs +++ b/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, - EstimateCallFee, ExecuteBlock, ExtrinsicCall, Get, GetBacking, GetDefault, HandleLifetime, - IsSubType, IsType, Len, OffchainWorker, OnKilledAccount, OnNewAccount, PrivilegeCmp, - SameOrOther, Time, TryCollect, TryDrop, TypedGet, UnixTime, WrapperKeepOpaque, WrapperOpaque, + ConstU32, ConstU64, ConstU8, DefensiveSaturating, DefensiveTruncateFrom, + EnsureInherentsAreFirst, EqualPrivilegeOnly, EstimateCallFee, ExecuteBlock, ExtrinsicCall, Get, + GetBacking, GetDefault, HandleLifetime, IsSubType, IsType, Len, OffchainWorker, + OnKilledAccount, OnNewAccount, PrivilegeCmp, SameOrOther, Time, TryCollect, TryDrop, TypedGet, + UnixTime, WrapperKeepOpaque, WrapperOpaque, }; #[allow(deprecated)] pub use misc::{PreimageProvider, PreimageRecipient}; diff --git a/substrate/frame/support/src/traits/misc.rs b/substrate/frame/support/src/traits/misc.rs index 5a976478fa7c4769a62861383091dadf172e6957..1297956b2f64ecf4af8bd60b71fc59bc6f4dd2fc 100644 --- a/substrate/frame/support/src/traits/misc.rs +++ b/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>), diff --git a/substrate/primitives/core/src/bounded/bounded_vec.rs b/substrate/primitives/core/src/bounded/bounded_vec.rs index 1832e43e8646c75cee43f2fec1c8dac983ab8180..2f39f3340ce5011073215640fc20fe2d8a4c4c8f 100644 --- a/substrate/primitives/core/src/bounded/bounded_vec.rs +++ b/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[..]); + } }