diff --git a/frame/beefy/src/lib.rs b/frame/beefy/src/lib.rs index fce531d3f5dd7..305b158124b67 100644 --- a/frame/beefy/src/lib.rs +++ b/frame/beefy/src/lib.rs @@ -159,7 +159,8 @@ impl Pallet { } let bounded_authorities = - BoundedSlice::::try_from(authorities.as_slice())?; + BoundedSlice::::try_from(authorities.as_slice()) + .map_err(|_| ())?; let id = 0; >::put(bounded_authorities); diff --git a/frame/preimage/src/lib.rs b/frame/preimage/src/lib.rs index e899d3643dbbf..6549832c11f5d 100644 --- a/frame/preimage/src/lib.rs +++ b/frame/preimage/src/lib.rs @@ -342,6 +342,7 @@ impl Pallet { fn insert(hash: &T::Hash, preimage: Cow<[u8]>) -> Result<(), ()> { BoundedSlice::>::try_from(preimage.as_ref()) + .map_err(|_| ()) .map(|s| PreimageFor::::insert((hash, s.len() as u32), s)) } diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 302d3354dae5e..2b6c5efee10bb 100644 --- a/frame/support/src/traits.rs +++ b/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/frame/support/src/traits/misc.rs b/frame/support/src/traits/misc.rs index 5a976478fa7c4..1297956b2f64e 100644 --- a/frame/support/src/traits/misc.rs +++ b/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 DefensiveSaturating f } } +/// Construct an object by defensively truncating an input if the `TryFrom` conversion fails. +pub trait DefensiveTruncateFrom { + /// 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::>::defensive_truncate_from(unbound); + /// + /// assert_eq!(bound, vec![1, 2]); + /// ``` + fn defensive_truncate_from(unbound: T) -> Self; +} + +impl DefensiveTruncateFrom 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 + TryFrom, +{ + 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 PreimageRecipient 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::>::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::>::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::>::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::>::defensive_truncate_from(unbound); + } + + #[test] + fn defensive_truncate_from_vec_works() { + let unbound = vec![1u32, 2, 3]; + let bound = BoundedVec::>::defensive_truncate_from(unbound.clone()); + assert_eq!(bound, unbound); + } + + #[test] + fn defensive_truncate_from_slice_works() { + let unbound = [1u32, 2, 3]; + let bound = BoundedSlice::>::defensive_truncate_from(&unbound); + assert_eq!(bound, &unbound[..]); + } + #[derive(Encode, Decode)] enum NestedType { Nested(Box), diff --git a/primitives/core/src/bounded/bounded_vec.rs b/primitives/core/src/bounded/bounded_vec.rs index 1832e43e8646c..2f39f3340ce50 100644 --- a/primitives/core/src/bounded/bounded_vec.rs +++ b/primitives/core/src/bounded/bounded_vec.rs @@ -47,6 +47,12 @@ pub struct BoundedVec( #[cfg_attr(feature = "std", serde(skip_serializing))] PhantomData, ); +/// Create an object through truncation. +pub trait TruncateFrom { + /// Create an object through truncation. + fn truncate_from(unbound: T) -> Self; +} + #[cfg(feature = "std")] impl<'de, T, S: Get> Deserialize<'de> for BoundedVec where @@ -234,12 +240,12 @@ impl<'a, T: Ord, Bound: Get> Ord for BoundedSlice<'a, T, Bound> { } impl<'a, T, S: Get> TryFrom<&'a [T]> for BoundedSlice<'a, T, S> { - type Error = (); + type Error = &'a [T]; fn try_from(t: &'a [T]) -> Result { if t.len() <= S::get() as usize { Ok(BoundedSlice(t, PhantomData)) } else { - Err(()) + Err(t) } } } @@ -250,12 +256,28 @@ impl<'a, T, S> From> for &'a [T] { } } +impl<'a, T, S: Get> TruncateFrom<&'a [T]> for BoundedSlice<'a, T, S> { + fn truncate_from(unbound: &'a [T]) -> Self { + BoundedSlice::::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, +{ + 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> TryFrom> for BoundedVec { } } +impl> TruncateFrom> for BoundedVec { + fn truncate_from(unbound: Vec) -> Self { + BoundedVec::::truncate_from(unbound) + } +} + // It is okay to give a non-mutable reference of the inner vec to anyone. impl AsRef> for BoundedVec { fn as_ref(&self) -> &Vec { @@ -809,6 +837,12 @@ where } } +impl<'a, T: PartialEq, S: Get> PartialEq<&'a [T]> for BoundedSlice<'a, T, S> { + fn eq(&self, other: &&'a [T]) -> bool { + &self.0 == other + } +} + impl> PartialEq> for BoundedVec { fn eq(&self, other: &Vec) -> bool { &self.0 == other @@ -1219,6 +1253,18 @@ pub mod test { assert!(b2.is_err()); } + #[test] + fn bounded_vec_debug_works() { + let bound = BoundedVec::>::truncate_from(vec![1, 2, 3]); + assert_eq!(format!("{:?}", bound), "BoundedVec([1, 2, 3], 5)"); + } + + #[test] + fn bounded_slice_debug_works() { + let bound = BoundedSlice::>::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> = 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::>::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::>::truncate_from(&unbound); + assert_eq!(bound, &[1, 2, 3][..]); + } + + #[test] + fn bounded_slice_partialeq_slice_works() { + let unbound = [1, 2, 3]; + let bound = BoundedSlice::>::truncate_from(&unbound); + + assert_eq!(bound, &unbound[..]); + assert!(bound == &unbound[..]); + } }