Identity should bound additional fields (paritytech#4770)

* Identity should bound additional fields

* ump rutnime

bin/node/runtime/src/lib.rs

@@ -80,8 +80,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// and set impl_version to 0. If only runtime
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
-	spec_version: 208,
-	impl_version: 1,
+	spec_version: 209,
+	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 };
 
@@ -490,7 +490,8 @@ parameter_types! {
 	pub const BasicDeposit: Balance = 10 * DOLLARS;       // 258 bytes on-chain
 	pub const FieldDeposit: Balance = 250 * CENTS;        // 66 bytes on-chain
 	pub const SubAccountDeposit: Balance = 2 * DOLLARS;   // 53 bytes on-chain
-	pub const MaximumSubAccounts: u32 = 100;
+	pub const MaxSubAccounts: u32 = 100;
+	pub const MaxAdditionalFields: u32 = 100;
 }
 
 impl pallet_identity::Trait for Runtime {
@@ -500,7 +501,8 @@ impl pallet_identity::Trait for Runtime {
 	type BasicDeposit = BasicDeposit;
 	type FieldDeposit = FieldDeposit;
 	type SubAccountDeposit = SubAccountDeposit;
-	type MaximumSubAccounts = MaximumSubAccounts;
+	type MaxSubAccounts = MaxSubAccounts;
+	type MaxAdditionalFields = MaxAdditionalFields;
 	type RegistrarOrigin = pallet_collective::EnsureProportionMoreThan<_1, _2, AccountId, CouncilCollective>;
 	type ForceOrigin = pallet_collective::EnsureProportionMoreThan<_1, _2, AccountId, CouncilCollective>;
 }

frame/identity/src/lib.rs

@@ -100,7 +100,11 @@ pub trait Trait: frame_system::Trait {
 	type SubAccountDeposit: Get<BalanceOf<Self>>;
 
 	/// The maximum number of sub-accounts allowed per identified account.
-	type MaximumSubAccounts: Get<u32>;
+	type MaxSubAccounts: Get<u32>;
+
+	/// Maximum number of additional fields that may be stored in an ID. Needed to bound the I/O
+	/// required to access an identity, but can be pretty high.
+	type MaxAdditionalFields: Get<u32>;
 
 	/// What to do with slashed funds.
 	type Slashed: OnUnbalanced<NegativeImbalanceOf<Self>>;
@@ -443,7 +447,9 @@ decl_error! {
 		InvalidIndex,
 		/// The target is invalid.
 		InvalidTarget,
-	}
+		/// Too many additional fields.
+		TooManyFields,
+}
 }
 
 decl_module! {
@@ -493,15 +499,17 @@ decl_module! {
 		/// Emits `IdentitySet` if successful.
 		///
 		/// # <weight>
-		/// - `O(X + R)` where `X` additional-field-count (deposit-bounded).
+		/// - `O(X + X' + R)` where `X` additional-field-count (deposit-bounded and code-bounded).
 		/// - At most two balance operations.
-		/// - One storage mutation (codec `O(X + R)`).
+		/// - One storage mutation (codec-read `O(X' + R)`, codec-write `O(X + R)`).
 		/// - One event.
 		/// # </weight>
 		#[weight = SimpleDispatchInfo::FixedNormal(50_000)]
 		fn set_identity(origin, info: IdentityInfo) {
 			let sender = ensure_signed(origin)?;
-			let fd = <BalanceOf<T>>::from(info.additional.len() as u32) * T::FieldDeposit::get();
+			let extra_fields = info.additional.len() as u32;
+			ensure!(extra_fields <= T::MaxAdditionalFields::get(), Error::<T>::TooManyFields);
+			let fd = <BalanceOf<T>>::from(extra_fields) * T::FieldDeposit::get();
 
 			let mut id = match <IdentityOf<T>>::get(&sender) {
 				Some(mut id) => {
@@ -546,7 +554,7 @@ decl_module! {
 		fn set_subs(origin, subs: Vec<(T::AccountId, Data)>) {
 			let sender = ensure_signed(origin)?;
 			ensure!(<IdentityOf<T>>::exists(&sender), Error::<T>::NotFound);
-			ensure!(subs.len() <= T::MaximumSubAccounts::get() as usize, Error::<T>::TooManySubAccounts);
+			ensure!(subs.len() <= T::MaxSubAccounts::get() as usize, Error::<T>::TooManySubAccounts);
 
 			let (old_deposit, old_ids) = <SubsOf<T>>::get(&sender);
 			let new_deposit = T::SubAccountDeposit::get() * <BalanceOf<T>>::from(subs.len() as u32);
@@ -936,7 +944,8 @@ mod tests {
 		pub const BasicDeposit: u64 = 10;
 		pub const FieldDeposit: u64 = 10;
 		pub const SubAccountDeposit: u64 = 10;
-		pub const MaximumSubAccounts: u32 = 2;
+		pub const MaxSubAccounts: u32 = 2;
+		pub const MaxAdditionalFields: u32 = 2;
 	}
 	ord_parameter_types! {
 		pub const One: u64 = 1;
@@ -949,7 +958,8 @@ mod tests {
 		type BasicDeposit = BasicDeposit;
 		type FieldDeposit = FieldDeposit;
 		type SubAccountDeposit = SubAccountDeposit;
-		type MaximumSubAccounts = MaximumSubAccounts;
+		type MaxSubAccounts = MaxSubAccounts;
+		type MaxAdditionalFields = MaxAdditionalFields;
 		type RegistrarOrigin = EnsureSignedBy<One, u64>;
 		type ForceOrigin = EnsureSignedBy<Two, u64>;
 	}
@@ -1012,6 +1022,14 @@ mod tests {
 		new_test_ext().execute_with(|| {
 			assert_ok!(Identity::add_registrar(Origin::signed(1), 3));
 			assert_ok!(Identity::set_fee(Origin::signed(3), 0, 10));
+			let mut three_fields = ten();
+			three_fields.additional.push(Default::default());
+			three_fields.additional.push(Default::default());
+			three_fields.additional.push(Default::default());
+			assert_noop!(
+				Identity::set_identity(Origin::signed(10), three_fields),
+				Error::<Test>::TooManyFields
+			);
 			assert_ok!(Identity::set_identity(Origin::signed(10), ten()));
 			assert_eq!(Identity::identity(10).unwrap().info, ten());
 			assert_eq!(Balances::free_balance(10), 90);