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 5 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
72 changes: 72 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,42 @@ impl<Hash> PreimageRecipient<Hash> for () {
#[cfg(test)]
mod test {
use super::*;
use sp_core::bounded::{BoundedSlice, BoundedVec};
use sp_std::marker::PhantomData;

#[test]
#[should_panic(
expected = "Defensive failure has been triggered!: \"DefensiveTruncateFrom truncating\""
)]
fn defensive_truncate_from_vec_panics() {
// NOTE: We cannot test the truncating case since defensive failures panic in tests.
let unbound = vec![1u32, 2];
let _ = BoundedVec::<u32, ConstU32<1>>::defensive_truncate_from(unbound);
}

#[test]
#[should_panic(
expected = "Defensive failure has been triggered!: \"DefensiveTruncateFrom truncating\""
)]
fn defensive_truncate_from_slice_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[..]);
}
}