Bounded for Sudo Test and Tips

frame/sudo/src/mock.rs

@@ -44,7 +44,6 @@ pub mod logger {
 
 	#[pallet::pallet]
 	#[pallet::generate_store(pub(super) trait Store)]
-	#[pallet::without_storage_info]
 	pub struct Pallet<T>(PhantomData<T>);
 
 	#[pallet::call]
@@ -57,7 +56,7 @@ pub mod logger {
 		) -> DispatchResultWithPostInfo {
 			// Ensure that the `origin` is `Root`.
 			ensure_root(origin)?;
-			<I32Log<T>>::append(i);
+			<I32Log<T>>::try_append(i).map_err(|_| "couldn't append")?;
 			Self::deposit_event(Event::AppendI32 { value: i, weight });
 			Ok(().into())
 		}
@@ -70,8 +69,8 @@ pub mod logger {
 		) -> DispatchResultWithPostInfo {
 			// Ensure that the `origin` is some signed account.
 			let sender = ensure_signed(origin)?;
-			<I32Log<T>>::append(i);
-			<AccountLog<T>>::append(sender.clone());
+			<I32Log<T>>::try_append(i).map_err(|_| "couldn't append")?;
+			<AccountLog<T>>::try_append(sender.clone()).map_err(|_| "couldn't append")?;
 			Self::deposit_event(Event::AppendI32AndAccount { sender, value: i, weight });
 			Ok(().into())
 		}
@@ -86,11 +85,12 @@ pub mod logger {
 
 	#[pallet::storage]
 	#[pallet::getter(fn account_log)]
-	pub(super) type AccountLog<T: Config> = StorageValue<_, Vec<T::AccountId>, ValueQuery>;
+	pub(super) type AccountLog<T: Config> =
+		StorageValue<_, BoundedVec<T::AccountId, ConstU32<1000>>, ValueQuery>;
 
 	#[pallet::storage]
 	#[pallet::getter(fn i32_log)]
-	pub(super) type I32Log<T> = StorageValue<_, Vec<i32>, ValueQuery>;
+	pub(super) type I32Log<T> = StorageValue<_, BoundedVec<i32, ConstU32<1000>>, ValueQuery>;
 }
 
 type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;

frame/support/src/traits.rs

@@ -35,8 +35,8 @@ mod members;
 pub use members::{AllowAll, DenyAll, Filter};
 pub use members::{
 	AsContains, ChangeMembers, Contains, ContainsLengthBound, ContainsPair, Everything,
-	EverythingBut, FromContainsPair, InitializeMembers, InsideBoth, IsInVec, Nothing,
-	SortedMembers, TheseExcept,
+	EverythingBut, FromContainsPair, InitializeMembers, InsideBoth, IsInVec, LengthBoundMaximum,
+	LengthBoundMinimum, Nothing, SortedMembers, TheseExcept,
 };
 
 mod validation;

frame/support/src/traits/members.rs

@@ -17,6 +17,7 @@
 
 //! Traits for dealing with the idea of membership.
 
+use sp_runtime::traits::Get;
 use sp_std::{marker::PhantomData, prelude::*};
 
 /// A trait for querying whether a type can be said to "contain" a value.
@@ -260,6 +261,24 @@ pub trait ContainsLengthBound {
 	fn max_len() -> usize;
 }
 
+/// A wrapper struct to convert an object which implements `ContainsLengthBound`, and returns
+/// the maximum length as a `Get<u32>`. Useful for implementing other bounding conditions.
+pub struct LengthBoundMaximum<T>(PhantomData<T>);
+impl<T: ContainsLengthBound> Get<u32> for LengthBoundMaximum<T> {
+	fn get() -> u32 {
+		T::max_len() as u32
+	}
+}
+
+/// A wrapper struct to convert an object which implements `ContainsLengthBound`, and returns
+/// the minimum length as a `Get<u32>`. Useful for implementing other bounding conditions.
+pub struct LengthBoundMinimum<T>(PhantomData<T>);
+impl<T: ContainsLengthBound> Get<u32> for LengthBoundMinimum<T> {
+	fn get() -> u32 {
+		T::min_len() as u32
+	}
+}
+
 /// Trait for type that can handle the initialization of account IDs at genesis.
 pub trait InitializeMembers<AccountId> {
 	/// Initialize the members to the given `members`.

frame/tips/src/lib.rs

@@ -66,13 +66,13 @@ use sp_runtime::{
 };
 use sp_std::prelude::*;
 
-use codec::{Decode, Encode};
+use codec::{Decode, Encode, MaxEncodedLen};
 use frame_support::{
 	traits::{
 		ContainsLengthBound, Currency, EnsureOrigin, ExistenceRequirement::KeepAlive, Get,
-		OnUnbalanced, ReservableCurrency, SortedMembers,
+		LengthBoundMaximum, OnUnbalanced, ReservableCurrency, SortedMembers,
 	},
-	Parameter,
+	BoundedVec, Parameter,
 };
 
 pub use pallet::*;
@@ -83,12 +83,17 @@ pub type NegativeImbalanceOf<T, I = ()> = pallet_treasury::NegativeImbalanceOf<T
 
 /// An open tipping "motion". Retains all details of a tip including information on the finder
 /// and the members who have voted.
-#[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug, scale_info::TypeInfo)]
+#[derive(
+	Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug, scale_info::TypeInfo, MaxEncodedLen,
+)]
+#[codec(mel_bound())]
+#[scale_info(skip_type_params(Tippers))]
 pub struct OpenTip<
-	AccountId: Parameter,
-	Balance: Parameter,
-	BlockNumber: Parameter,
-	Hash: Parameter,
+	AccountId: Parameter + MaxEncodedLen,
+	Balance: Parameter + MaxEncodedLen,
+	BlockNumber: Parameter + MaxEncodedLen,
+	Hash: Parameter + MaxEncodedLen,
+	Tippers: ContainsLengthBound,
 > {
 	/// The hash of the reason for the tip. The reason should be a human-readable UTF-8 encoded
 	/// string. A URL would be sensible.
@@ -103,7 +108,7 @@ pub struct OpenTip<
 	/// scheduled.
 	closes: Option<BlockNumber>,
 	/// The members who have voted for this tip. Sorted by AccountId.
-	tips: Vec<(AccountId, Balance)>,
+	tips: BoundedVec<(AccountId, Balance), LengthBoundMaximum<Tippers>>,
 	/// Whether this tip should result in the finder taking a fee.
 	finders_fee: bool,
 }
@@ -120,7 +125,6 @@ pub mod pallet {
 	#[pallet::pallet]
 	#[pallet::generate_store(pub(super) trait Store)]
 	#[pallet::storage_version(STORAGE_VERSION)]
-	#[pallet::without_storage_info]
 	pub struct Pallet<T, I = ()>(_);
 
 	#[pallet::config]
@@ -170,7 +174,7 @@ pub mod pallet {
 		_,
 		Twox64Concat,
 		T::Hash,
-		OpenTip<T::AccountId, BalanceOf<T, I>, T::BlockNumber, T::Hash>,
+		OpenTip<T::AccountId, BalanceOf<T, I>, T::BlockNumber, T::Hash, T::Tippers>,
 		OptionQuery,
 	>;
 
@@ -179,7 +183,7 @@ pub mod pallet {
 	#[pallet::storage]
 	#[pallet::getter(fn reasons)]
 	pub type Reasons<T: Config<I>, I: 'static = ()> =
-		StorageMap<_, Identity, T::Hash, Vec<u8>, OptionQuery>;
+		StorageMap<_, Identity, T::Hash, BoundedVec<u8, T::MaximumReasonLength>, OptionQuery>;
 
 	#[pallet::event]
 	#[pallet::generate_deposit(pub(super) fn deposit_event)]
@@ -210,6 +214,8 @@ pub mod pallet {
 		StillOpen,
 		/// The tip cannot be claimed/closed because it's still in the countdown period.
 		Premature,
+		/// Too many tips are being stored.
+		TipsTooBig,
 	}
 
 	#[pallet::call]
@@ -241,10 +247,9 @@ pub mod pallet {
 		) -> DispatchResult {
 			let finder = ensure_signed(origin)?;
 
-			ensure!(
-				reason.len() <= T::MaximumReasonLength::get() as usize,
-				Error::<T, I>::ReasonTooBig
-			);
+			// make reason bounded
+			let reason: BoundedVec<u8, _> =
+				reason.try_into().map_err(|_| Error::<T, I>::ReasonTooBig)?;
 
 			let reason_hash = T::Hashing::hash(&reason[..]);
 			ensure!(!Reasons::<T, I>::contains_key(&reason_hash), Error::<T, I>::AlreadyKnown);
@@ -262,7 +267,7 @@ pub mod pallet {
 				finder,
 				deposit,
 				closes: None,
-				tips: vec![],
+				tips: BoundedVec::default(),
 				finders_fee: true,
 			};
 			Tips::<T, I>::insert(&hash, tip);
@@ -334,6 +339,10 @@ pub mod pallet {
 			who: T::AccountId,
 			#[pallet::compact] tip_value: BalanceOf<T, I>,
 		) -> DispatchResult {
+			// make reason bounded
+			let reason: BoundedVec<u8, _> =
+				reason.try_into().map_err(|_| Error::<T, I>::ReasonTooBig)?;
+
 			let tipper = ensure_signed(origin)?;
 			ensure!(T::Tippers::contains(&tipper), BadOrigin);
 			let reason_hash = T::Hashing::hash(&reason[..]);
@@ -342,7 +351,9 @@ pub mod pallet {
 
 			Reasons::<T, I>::insert(&reason_hash, &reason);
 			Self::deposit_event(Event::NewTip { tip_hash: hash });
-			let tips = vec![(tipper.clone(), tip_value)];
+			let tips = vec![(tipper.clone(), tip_value)]
+				.try_into()
+				.map_err(|_| Error::<T, I>::TipsTooBig)?;
 			let tip = OpenTip {
 				reason: reason_hash,
 				who,
@@ -390,7 +401,7 @@ pub mod pallet {
 			ensure!(T::Tippers::contains(&tipper), BadOrigin);
 
 			let mut tip = Tips::<T, I>::get(hash).ok_or(Error::<T, I>::UnknownTip)?;
-			if Self::insert_tip_and_check_closing(&mut tip, tipper, tip_value) {
+			if Self::insert_tip_and_check_closing(&mut tip, tipper, tip_value)? {
 				Self::deposit_event(Event::TipClosing { tip_hash: hash });
 			}
 			Tips::<T, I>::insert(&hash, tip);
@@ -476,26 +487,31 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	///
 	/// `O(T)` and one storage access.
 	fn insert_tip_and_check_closing(
-		tip: &mut OpenTip<T::AccountId, BalanceOf<T, I>, T::BlockNumber, T::Hash>,
+		tip: &mut OpenTip<T::AccountId, BalanceOf<T, I>, T::BlockNumber, T::Hash, T::Tippers>,
 		tipper: T::AccountId,
 		tip_value: BalanceOf<T, I>,
-	) -> bool {
+	) -> Result<bool, sp_runtime::DispatchError> {
 		match tip.tips.binary_search_by_key(&&tipper, |x| &x.0) {
 			Ok(pos) => tip.tips[pos] = (tipper, tip_value),
-			Err(pos) => tip.tips.insert(pos, (tipper, tip_value)),
+			Err(pos) => tip
+				.tips
+				.try_insert(pos, (tipper, tip_value))
+				.map_err(|_| Error::<T, I>::TipsTooBig)?,
 		}
 		Self::retain_active_tips(&mut tip.tips);
 		let threshold = (T::Tippers::count() + 1) / 2;
 		if tip.tips.len() >= threshold && tip.closes.is_none() {
 			tip.closes = Some(frame_system::Pallet::<T>::block_number() + T::TipCountdown::get());
-			true
+			Ok(true)
 		} else {
-			false
+			Ok(false)
 		}
 	}
 
 	/// Remove any non-members of `Tippers` from a `tips` vector. `O(T)`.
-	fn retain_active_tips(tips: &mut Vec<(T::AccountId, BalanceOf<T, I>)>) {
+	fn retain_active_tips(
+		tips: &mut BoundedVec<(T::AccountId, BalanceOf<T, I>), LengthBoundMaximum<T::Tippers>>,
+	) {
 		let members = T::Tippers::sorted_members();
 		let mut members_iter = members.iter();
 		let mut member = members_iter.next();
@@ -521,7 +537,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	/// Plus `O(T)` (`T` is Tippers length).
 	fn payout_tip(
 		hash: T::Hash,
-		tip: OpenTip<T::AccountId, BalanceOf<T, I>, T::BlockNumber, T::Hash>,
+		tip: OpenTip<T::AccountId, BalanceOf<T, I>, T::BlockNumber, T::Hash, T::Tippers>,
 	) {
 		let mut tips = tip.tips;
 		Self::retain_active_tips(&mut tips);
@@ -552,7 +568,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		Self::deposit_event(Event::TipClosed { tip_hash: hash, who: tip.who, payout });
 	}
 
-	pub fn migrate_retract_tip_for_tip_new(module: &[u8], item: &[u8]) {
+	pub fn migrate_retract_tip_for_tip_new(module: &[u8], item: &[u8]) -> Result<(), ()> {
 		/// An open tipping "motion". Retains all details of a tip including information on the
 		/// finder and the members who have voted.
 		#[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug)]
@@ -598,10 +614,12 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				finder,
 				deposit,
 				closes: old_tip.closes,
-				tips: old_tip.tips,
+				tips: BoundedVec::try_from(old_tip.tips).map_err(|_| ())?,
 				finders_fee,
 			};
 			Tips::<T, I>::insert(hash, new_tip)
 		}
+
+		Ok(())
 	}
 }

frame/tips/src/tests.rs

@@ -30,7 +30,7 @@ use sp_runtime::{
 use sp_storage::Storage;
 
 use frame_support::{
-	assert_noop, assert_ok,
+	assert_noop, assert_ok, bounded_vec,
 	pallet_prelude::GenesisBuild,
 	parameter_types,
 	storage::StoragePrefixedMap,
@@ -103,6 +103,7 @@ impl pallet_balances::Config for Test {
 thread_local! {
 	static TEN_TO_FOURTEEN: RefCell<Vec<u128>> = RefCell::new(vec![10,11,12,13,14]);
 }
+#[derive(PartialEq, Debug)]
 pub struct TenToFourteen;
 impl SortedMembers<u128> for TenToFourteen {
 	fn sorted_members() -> Vec<u128> {
@@ -470,7 +471,7 @@ fn test_last_reward_migration() {
 	sp_io::TestExternalities::new(s).execute_with(|| {
 		let module = pallet_tips::Tips::<Test>::module_prefix();
 		let item = pallet_tips::Tips::<Test>::storage_prefix();
-		Tips::migrate_retract_tip_for_tip_new(module, item);
+		Tips::migrate_retract_tip_for_tip_new(module, item).unwrap();
 
 		// Test w/ finder
 		assert_eq!(
@@ -481,7 +482,7 @@ fn test_last_reward_migration() {
 				finder: 20,
 				deposit: 30,
 				closes: Some(13),
-				tips: vec![(40, 50), (60, 70)],
+				tips: bounded_vec![(40, 50), (60, 70)],
 				finders_fee: true,
 			})
 		);
@@ -495,7 +496,7 @@ fn test_last_reward_migration() {
 				finder: Default::default(),
 				deposit: 0,
 				closes: Some(13),
-				tips: vec![(40, 50), (60, 70)],
+				tips: bounded_vec![(40, 50), (60, 70)],
 				finders_fee: false,
 			})
 		);
@@ -507,13 +508,13 @@ fn test_migration_v4() {
 	let reason1 = BlakeTwo256::hash(b"reason1");
 	let hash1 = BlakeTwo256::hash_of(&(reason1, 10u64));
 
-	let tip = OpenTip::<u128, u64, u64, H256> {
+	let tip = OpenTip::<u128, u64, u64, H256, TenToFourteen> {
 		reason: reason1,
 		who: 10,
 		finder: 20,
 		deposit: 30,
 		closes: Some(13),
-		tips: vec![(40, 50), (60, 70)],
+		tips: bounded_vec![(40, 50), (60, 70)],
 		finders_fee: true,
 	};
 

primitives/runtime/src/bounded/bounded_vec.rs

@@ -273,6 +273,17 @@ impl<T, S> BoundedVec<T, S> {
 		self.0
 	}
 
+	/// Exactly the same semantics as [`slice::sort_by_key`].
+	///
+	/// This is safe since sorting cannot change the number of elements in the vector.
+	pub fn sort_by_key<K, F>(&mut self, f: F)
+	where
+		F: FnMut(&T) -> K,
+		K: Ord,
+	{
+		self.0.sort_by_key(f)
+	}
+
 	/// Exactly the same semantics as [`slice::sort_by`].
 	///
 	/// This is safe since sorting cannot change the number of elements in the vector.