Add ensure_council_election_parameters_valid() to the codex

runtime-modules/proposals/codex/src/lib.rs

@@ -56,7 +56,7 @@ use rstd::prelude::*;
 use rstd::str::from_utf8;
 use rstd::vec::Vec;
 use runtime_io::blake2_256;
-use sr_primitives::traits::Zero;
+use sr_primitives::traits::{One, Zero};
 use srml_support::dispatch::DispatchResult;
 use srml_support::traits::{Currency, Get};
 use srml_support::{decl_error, decl_module, decl_storage, ensure, print};
@@ -160,11 +160,33 @@ decl_error! {
         /// Invalid storage role parameter - min_service_period
         InvalidStorageRoleParameterMinServicePeriod,
 
-        /// Invalid storage role parameter - min_service_period
-        InvalidStorageRoleParameterMinServicePeriod,
-
         /// Invalid storage role parameter - startup_grace_period
         InvalidStorageRoleParameterStartupGracePeriod,
+
+        /// Invalid council election parameter - council_size
+        InvalidCouncilElectionParameterCouncilSize,
+
+        /// Invalid council election parameter - candidacy-limit
+        InvalidCouncilElectionParameterCandidacyLimit,
+
+        /// Invalid council election parameter - min-voting_stake
+        InvalidCouncilElectionParameterMinVotingStake,
+
+        /// Invalid council election parameter - new_term_duration
+        InvalidCouncilElectionParameterNewTermDuration,
+
+        /// Invalid council election parameter - min_council_stake
+        InvalidCouncilElectionParameterMinCouncilStake,
+
+        /// Invalid council election parameter - revealing_period
+        InvalidCouncilElectionParameterRevealingPeriod,
+
+        /// Invalid council election parameter - voting_period
+        InvalidCouncilElectionParameterVotingPeriod,
+
+        /// Invalid council election parameter - announcing_period
+        InvalidCouncilElectionParameterAnnouncingPeriod,
+
     }
 }
 
@@ -300,7 +322,7 @@ decl_module! {
             stake_balance: Option<BalanceOf<T>>,
             election_parameters: ElectionParameters<BalanceOfGovernanceCurrency<T>, T::BlockNumber>,
         ) {
-            election_parameters.ensure_valid()?;
+            Self::ensure_council_election_parameters_valid(&election_parameters)?;
 
             let proposal_code =
                 <governance::election::Call<T>>::set_election_parameters(election_parameters.clone());
@@ -679,4 +701,51 @@ impl<T: Trait> Module<T> {
 
         Ok(())
     }
+
+    // validates council election parameters for the 'Set election parameters' proposal
+    fn ensure_council_election_parameters_valid(
+        election_parameters: &ElectionParameters<BalanceOfGovernanceCurrency<T>, T::BlockNumber>,
+    ) -> Result<(), Error> {
+        ensure!(
+            election_parameters.council_size >= 3,
+            Error::InvalidCouncilElectionParameterCouncilSize
+        );
+
+        ensure!(
+            election_parameters.candidacy_limit >= 25,
+            Error::InvalidCouncilElectionParameterCandidacyLimit
+        );
+
+        ensure!(
+            election_parameters.min_voting_stake >= <BalanceOfGovernanceCurrency<T>>::one(),
+            Error::InvalidCouncilElectionParameterMinVotingStake
+        );
+
+        ensure!(
+            election_parameters.new_term_duration >= T::BlockNumber::from(14400),
+            Error::InvalidCouncilElectionParameterNewTermDuration
+        );
+
+        ensure!(
+            election_parameters.revealing_period >= T::BlockNumber::from(14400),
+            Error::InvalidCouncilElectionParameterRevealingPeriod
+        );
+
+        ensure!(
+            election_parameters.voting_period >= T::BlockNumber::from(14400),
+            Error::InvalidCouncilElectionParameterVotingPeriod
+        );
+
+        ensure!(
+            election_parameters.announcing_period >= T::BlockNumber::from(14400),
+            Error::InvalidCouncilElectionParameterAnnouncingPeriod
+        );
+
+        ensure!(
+            election_parameters.min_council_stake >= <BalanceOfGovernanceCurrency<T>>::one(),
+            Error::InvalidCouncilElectionParameterMinCouncilStake
+        );
+
+        Ok(())
+    }
 }

runtime-modules/proposals/codex/src/tests/mock.rs

@@ -123,8 +123,10 @@ impl governance::council::Trait for Test {
 }
 
 impl common::origin_validator::ActorOriginValidator<Origin, u64, u64> for () {
-    fn ensure_actor_origin(_: Origin, _: u64) -> Result<u64, &'static str> {
-        Ok(1)
+    fn ensure_actor_origin(origin: Origin, _: u64) -> Result<u64, &'static str> {
+        let account_id = system::ensure_signed(origin)?;
+
+        Ok(account_id)
     }
 }
 

runtime-modules/proposals/codex/src/tests/mod.rs

@@ -45,7 +45,10 @@ where
     }
 
     fn check_call_for_insufficient_rights(&self) {
-        assert!((self.insufficient_rights_call)().is_err());
+        assert_eq!(
+            (self.insufficient_rights_call)(),
+            Err(Error::Other("RequireSignedOrigin"))
+        );
     }
 
     fn check_for_successful_call(&self) {
@@ -261,26 +264,15 @@ fn create_upgrade_runtime_proposal_codex_call_fails_with_not_allowed_member_id()
 #[test]
 fn create_set_election_parameters_proposal_common_checks_succeed() {
     initial_test_ext().execute_with(|| {
-        let election_parameters = ElectionParameters {
-            announcing_period: 1,
-            voting_period: 2,
-            revealing_period: 3,
-            council_size: 4,
-            candidacy_limit: 5,
-            min_voting_stake: 6,
-            min_council_stake: 7,
-            new_term_duration: 8,
-        };
-
         let proposal_fixture = ProposalTestFixture {
             insufficient_rights_call: || {
                 ProposalCodex::create_set_election_parameters_proposal(
-                    RawOrigin::Signed(1).into(),
+                    RawOrigin::None.into(),
                     1,
                     b"title".to_vec(),
                     b"body".to_vec(),
-                    Some(<BalanceOf<Test>>::from(500u32)),
-                    ElectionParameters::default(),
+                    None,
+                    get_valid_election_parameters(),
                 )
             },
             empty_stake_call: || {
@@ -290,7 +282,7 @@ fn create_set_election_parameters_proposal_common_checks_succeed() {
                     b"title".to_vec(),
                     b"body".to_vec(),
                     None,
-                    election_parameters.clone(),
+                    get_valid_election_parameters(),
                 )
             },
             invalid_stake_call: || {
@@ -300,7 +292,7 @@ fn create_set_election_parameters_proposal_common_checks_succeed() {
                     b"title".to_vec(),
                     b"body".to_vec(),
                     Some(<BalanceOf<Test>>::from(50000u32)),
-                    election_parameters.clone(),
+                    get_valid_election_parameters(),
                 )
             },
             successful_call: || {
@@ -310,33 +302,108 @@ fn create_set_election_parameters_proposal_common_checks_succeed() {
                     b"title".to_vec(),
                     b"body".to_vec(),
                     Some(<BalanceOf<Test>>::from(500u32)),
-                    election_parameters.clone(),
+                    get_valid_election_parameters(),
                 )
             },
             proposal_parameters:
                 crate::proposal_types::parameters::set_election_parameters_proposal::<Test>(),
-            proposal_details: ProposalDetails::SetElectionParameters(election_parameters),
+            proposal_details: ProposalDetails::SetElectionParameters(
+                get_valid_election_parameters(),
+            ),
         };
         proposal_fixture.check_all();
     });
 }
+
+fn assert_failed_election_parameters_call(
+    election_parameters: ElectionParameters<u64, u64>,
+    error: Error,
+) {
+    assert_eq!(
+        ProposalCodex::create_set_election_parameters_proposal(
+            RawOrigin::Signed(1).into(),
+            1,
+            b"title".to_vec(),
+            b"body".to_vec(),
+            Some(<BalanceOf<Test>>::from(500u32)),
+            election_parameters,
+        ),
+        Err(error)
+    );
+}
+
+fn get_valid_election_parameters() -> ElectionParameters<u64, u64> {
+    ElectionParameters {
+        announcing_period: 14400,
+        voting_period: 14400,
+        revealing_period: 14400,
+        council_size: 3,
+        candidacy_limit: 25,
+        new_term_duration: 14400,
+        min_council_stake: 1,
+        min_voting_stake: 1,
+    }
+}
+
 #[test]
 fn create_set_election_parameters_call_fails_with_incorrect_parameters() {
     initial_test_ext().execute_with(|| {
-        let account_id = 1;
-        let required_stake = Some(<BalanceOf<Test>>::from(500u32));
-        let _imbalance = <Test as stake::Trait>::Currency::deposit_creating(&account_id, 50000);
+        let _imbalance = <Test as stake::Trait>::Currency::deposit_creating(&1, 50000);
 
-        assert_eq!(
-            ProposalCodex::create_set_election_parameters_proposal(
-                RawOrigin::Signed(1).into(),
-                1,
-                b"title".to_vec(),
-                b"body".to_vec(),
-                required_stake,
-                ElectionParameters::default(),
-            ),
-            Err(Error::Other("PeriodCannotBeZero"))
+        let mut election_parameters = get_valid_election_parameters();
+        election_parameters.council_size = 2;
+        assert_failed_election_parameters_call(
+            election_parameters,
+            Error::InvalidCouncilElectionParameterCouncilSize,
+        );
+
+        election_parameters = get_valid_election_parameters();
+        election_parameters.candidacy_limit = 22;
+        assert_failed_election_parameters_call(
+            election_parameters,
+            Error::InvalidCouncilElectionParameterCandidacyLimit,
+        );
+
+        election_parameters = get_valid_election_parameters();
+        election_parameters.min_voting_stake = 0;
+        assert_failed_election_parameters_call(
+            election_parameters,
+            Error::InvalidCouncilElectionParameterMinVotingStake,
+        );
+
+        election_parameters = get_valid_election_parameters();
+        election_parameters.new_term_duration = 10000;
+        assert_failed_election_parameters_call(
+            election_parameters,
+            Error::InvalidCouncilElectionParameterNewTermDuration,
+        );
+
+        election_parameters = get_valid_election_parameters();
+        election_parameters.min_council_stake = 0;
+        assert_failed_election_parameters_call(
+            election_parameters,
+            Error::InvalidCouncilElectionParameterMinCouncilStake,
+        );
+
+        election_parameters = get_valid_election_parameters();
+        election_parameters.voting_period = 10000;
+        assert_failed_election_parameters_call(
+            election_parameters,
+            Error::InvalidCouncilElectionParameterVotingPeriod,
+        );
+
+        election_parameters = get_valid_election_parameters();
+        election_parameters.revealing_period = 10000;
+        assert_failed_election_parameters_call(
+            election_parameters,
+            Error::InvalidCouncilElectionParameterRevealingPeriod,
+        );
+
+        election_parameters = get_valid_election_parameters();
+        election_parameters.announcing_period = 10000;
+        assert_failed_election_parameters_call(
+            election_parameters,
+            Error::InvalidCouncilElectionParameterAnnouncingPeriod,
         );
     });
 }