Browse Source

runtime: pallet_common: Refactor InputValidationLengthConstraint.

Shamil Gadelshin 3 years ago
parent
commit
decd9984ad

+ 2 - 0
Cargo.lock

@@ -3818,7 +3818,9 @@ dependencies = [
  "pallet-timestamp",
  "parity-scale-codec",
  "serde",
+ "sp-arithmetic",
  "sp-runtime",
+ "sp-std",
  "strum 0.19.5",
  "strum_macros 0.19.4",
 ]

+ 4 - 0
runtime-modules/common/Cargo.toml

@@ -13,6 +13,8 @@ sp-runtime = { package = 'sp-runtime', default-features = false, git = 'https://
 frame-support = { package = 'frame-support', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = '2cd20966cc09b059817c3ebe12fc130cdd850d62'}
 frame-system = { package = 'frame-system', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = '2cd20966cc09b059817c3ebe12fc130cdd850d62'}
 pallet-timestamp = { package = 'pallet-timestamp', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = '2cd20966cc09b059817c3ebe12fc130cdd850d62'}
+sp-arithmetic = { package = 'sp-arithmetic', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = '2cd20966cc09b059817c3ebe12fc130cdd850d62'}
+sp-std = { package = 'sp-std', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = '2cd20966cc09b059817c3ebe12fc130cdd850d62'}
 
 [features]
 default = ['std']
@@ -25,4 +27,6 @@ std = [
 	'frame-support/std',
 	'frame-system/std',
 	'pallet-timestamp/std',
+	'sp-arithmetic/std',
+	'sp-std/std',
 ]

+ 30 - 23
runtime-modules/common/src/constraints.rs

@@ -1,45 +1,52 @@
 use codec::{Decode, Encode};
 #[cfg(feature = "std")]
 use serde::{Deserialize, Serialize};
+use sp_arithmetic::traits::UniqueSaturatedInto;
+use sp_std::ops::Add;
 
-/// Length constraint for input validation
+/// Length constraint for input validation.
+pub type InputValidationLengthConstraint = BoundedValueConstraint<u16>;
+
+/// Value constraint within boundaries.
 #[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))]
 #[derive(Encode, Decode, Default, Clone, PartialEq, Eq, Copy)]
-pub struct InputValidationLengthConstraint {
-    /// Minimum length
-    pub min: u16,
+pub struct BoundedValueConstraint<Val> {
+    /// Minimum value
+    pub min: Val,
 
-    /// Difference between minimum length and max length.
+    /// Difference between minimum value and max value.
     /// While having max would have been more direct, this
     /// way makes max < min unrepresentable semantically,
     /// which is safer.
-    pub max_min_diff: u16,
+    pub max_min_diff: Val,
 }
 
-impl InputValidationLengthConstraint {
-    /// Helper for computing max
-    pub fn max(self) -> u16 {
+impl<Val: Copy + Add<Val, Output = Val> + Ord> BoundedValueConstraint<Val> {
+    /// Helper for computing max value.
+    pub fn max(&self) -> Val {
         self.min + self.max_min_diff
     }
 
-    pub fn ensure_valid(
-        self,
-        len: usize,
-        too_short_msg: &'static str,
-        too_long_msg: &'static str,
-    ) -> Result<(), &'static str> {
-        let length = len as u16;
-        if length < self.min {
-            Err(too_short_msg)
-        } else if length > self.max() {
-            Err(too_long_msg)
+    /// Validates value and its constraints.
+    pub fn ensure_valid<E, ValType: UniqueSaturatedInto<Val>>(
+        &self,
+        value: ValType,
+        less_err: E,
+        greater_err: E,
+    ) -> Result<(), E> {
+        let converted_value: Val = value.unique_saturated_into();
+
+        if converted_value < self.min {
+            Err(less_err)
+        } else if converted_value > self.max() {
+            Err(greater_err)
         } else {
             Ok(())
         }
     }
 
-    /// Create default input text constraints.
-    pub fn new(min: u16, max_min_diff: u16) -> Self {
-        InputValidationLengthConstraint { min, max_min_diff }
+    /// Create new value constraints.
+    pub fn new(min: Val, max_min_diff: Val) -> Self {
+        Self { min, max_min_diff }
     }
 }

+ 15 - 21
runtime-modules/working-group/src/lib.rs

@@ -1166,13 +1166,11 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
     }
 
     fn ensure_opening_human_readable_text_is_valid(text: &[u8]) -> DispatchResult {
-        <OpeningHumanReadableText<I>>::get()
-            .ensure_valid(
-                text.len(),
-                Error::<T, I>::OpeningTextTooShort.into(),
-                Error::<T, I>::OpeningTextTooLong.into(),
-            )
-            .map_err(|e| DispatchError::Other(e))
+        <OpeningHumanReadableText<I>>::get().ensure_valid(
+            text.len(),
+            Error::<T, I>::OpeningTextTooShort.into(),
+            Error::<T, I>::OpeningTextTooLong.into(),
+        )
     }
 
     /// Ensures origin is signed by the leader.
@@ -1218,13 +1216,11 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
     }
 
     fn ensure_application_text_is_valid(text: &[u8]) -> DispatchResult {
-        <WorkerApplicationHumanReadableText<I>>::get()
-            .ensure_valid(
-                text.len(),
-                Error::<T, I>::WorkerApplicationTextTooShort.into(),
-                Error::<T, I>::WorkerApplicationTextTooLong.into(),
-            )
-            .map_err(|e| DispatchError::Other(e))
+        <WorkerApplicationHumanReadableText<I>>::get().ensure_valid(
+            text.len(),
+            Error::<T, I>::WorkerApplicationTextTooShort.into(),
+            Error::<T, I>::WorkerApplicationTextTooLong.into(),
+        )
     }
 
     // CRITICAL:
@@ -1326,13 +1322,11 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
     }
 
     fn ensure_worker_exit_rationale_text_is_valid(text: &[u8]) -> DispatchResult {
-        Self::worker_exit_rationale_text()
-            .ensure_valid(
-                text.len(),
-                Error::<T, I>::WorkerExitRationaleTextTooShort.into(),
-                Error::<T, I>::WorkerExitRationaleTextTooLong.into(),
-            )
-            .map_err(|e| DispatchError::Other(e))
+        Self::worker_exit_rationale_text().ensure_valid(
+            text.len(),
+            Error::<T, I>::WorkerExitRationaleTextTooShort.into(),
+            Error::<T, I>::WorkerExitRationaleTextTooLong.into(),
+        )
     }
 }
 

+ 18 - 10
runtime-modules/working-group/src/tests/mod.rs

@@ -253,12 +253,16 @@ fn add_opening_fails_with_invalid_human_readable_text() {
 
         let add_opening_fixture = AddWorkerOpeningFixture::default().with_text(Vec::new());
 
-        add_opening_fixture.call_and_assert(Err(DispatchError::Other("OpeningTextTooShort")));
+        add_opening_fixture.call_and_assert(Err(
+            Error::<Test, TestWorkingGroupInstance>::OpeningTextTooShort.into(),
+        ));
 
         let add_opening_fixture =
             AddWorkerOpeningFixture::default().with_text(b"Long text".to_vec());
 
-        add_opening_fixture.call_and_assert(Err(DispatchError::Other("OpeningTextTooLong")));
+        add_opening_fixture.call_and_assert(Err(
+            Error::<Test, TestWorkingGroupInstance>::OpeningTextTooLong.into(),
+        ));
     });
 }
 
@@ -537,14 +541,16 @@ fn apply_on_opening_fails_with_invalid_text() {
 
         let apply_on_opening_fixture =
             ApplyOnWorkerOpeningFixture::default_for_opening_id(opening_id).with_text(Vec::new());
-        apply_on_opening_fixture
-            .call_and_assert(Err(DispatchError::Other("WorkerApplicationTextTooShort")));
+        apply_on_opening_fixture.call_and_assert(Err(
+            Error::<Test, TestWorkingGroupInstance>::WorkerApplicationTextTooShort.into(),
+        ));
 
         let apply_on_opening_fixture =
             ApplyOnWorkerOpeningFixture::default_for_opening_id(opening_id)
                 .with_text(b"Long text".to_vec());
-        apply_on_opening_fixture
-            .call_and_assert(Err(DispatchError::Other("WorkerApplicationTextTooLong")));
+        apply_on_opening_fixture.call_and_assert(Err(
+            Error::<Test, TestWorkingGroupInstance>::WorkerApplicationTextTooLong.into(),
+        ));
     });
 }
 
@@ -1748,14 +1754,16 @@ fn terminate_worker_role_fails_with_invalid_text() {
 
         let terminate_worker_role_fixture =
             TerminateWorkerRoleFixture::default_for_worker_id(worker_id).with_text(Vec::new());
-        terminate_worker_role_fixture
-            .call_and_assert(Err(DispatchError::Other("WorkerExitRationaleTextTooShort")));
+        terminate_worker_role_fixture.call_and_assert(Err(
+            Error::<Test, TestWorkingGroupInstance>::WorkerExitRationaleTextTooShort.into(),
+        ));
 
         let terminate_worker_role_fixture =
             TerminateWorkerRoleFixture::default_for_worker_id(worker_id)
                 .with_text(b"MSG_WORKER_EXIT_RATIONALE_TEXT_TOO_LONG".to_vec());
-        terminate_worker_role_fixture
-            .call_and_assert(Err(DispatchError::Other("WorkerExitRationaleTextTooLong")));
+        terminate_worker_role_fixture.call_and_assert(Err(
+            Error::<Test, TestWorkingGroupInstance>::WorkerExitRationaleTextTooLong.into(),
+        ));
     });
 }