Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add DefensiveTruncateFrom #12515

Merged
merged 8 commits into from
Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion frame/beefy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions frame/preimage/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down
9 changes: 5 additions & 4 deletions frame/support/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
89 changes: 89 additions & 0 deletions frame/support/src/traits/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
// `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.
Expand Down Expand Up @@ -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>),
Expand Down
73 changes: 71 additions & 2 deletions primitives/core/src/bounded/bounded_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
}
Expand All @@ -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> {}

Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1219,11 +1253,46 @@ 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];
// Sort by absolute value.
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[..]);
}
}