Migrate proposals engine to decl_error! macro

- introduce errors in decl_error! macro for the crate
- delete errors module
- fix tests

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

@@ -47,7 +47,7 @@ fn create_text_proposal_codex_call_fails_with_invalid_stake() {
                 b"text".to_vec(),
                 None,
             ),
-            Err(Error::Other("Stake cannot be empty with this proposal"))
+            Err(Error::Other("EmptyStake"))
         );
 
         let invalid_stake = Some(<BalanceOf<Test>>::from(5000u32));
@@ -61,7 +61,7 @@ fn create_text_proposal_codex_call_fails_with_invalid_stake() {
                 b"text".to_vec(),
                 invalid_stake,
             ),
-            Err(Error::Other("Stake differs from the proposal requirements"))
+            Err(Error::Other("StakeDiffersFromRequired"))
         );
     });
 }
@@ -177,7 +177,7 @@ fn create_runtime_upgrade_proposal_codex_call_fails_with_invalid_stake() {
                 b"wasm".to_vec(),
                 None,
             ),
-            Err(Error::Other("Stake cannot be empty with this proposal"))
+            Err(Error::Other("EmptyStake"))
         );
 
         let invalid_stake = Some(<BalanceOf<Test>>::from(500u32));
@@ -191,7 +191,7 @@ fn create_runtime_upgrade_proposal_codex_call_fails_with_invalid_stake() {
                 b"wasm".to_vec(),
                 invalid_stake,
             ),
-            Err(Error::Other("Stake differs from the proposal requirements"))
+            Err(Error::Other("StakeDiffersFromRequired"))
         );
     });
 }

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

@@ -35,7 +35,6 @@ pub use types::{DefaultStakeHandlerProvider, StakeHandler, StakeHandlerProvider}
 pub use types::{ProposalCodeDecoder, ProposalExecutable};
 pub use types::{VoteKind, VotersParameters};
 
-mod errors;
 pub(crate) mod types;
 
 #[cfg(test)]
@@ -43,10 +42,10 @@ mod tests;
 
 use codec::Decode;
 use rstd::prelude::*;
-use sr_primitives::traits::Zero;
+use sr_primitives::traits::{DispatchResult, Zero};
 use srml_support::traits::{Currency, Get};
 use srml_support::{
-    decl_event, decl_module, decl_storage, dispatch, ensure, Parameter, StorageDoubleMap,
+    decl_error, decl_event, decl_module, decl_storage, ensure, Parameter, StorageDoubleMap,
 };
 use system::{ensure_root, RawOrigin};
 
@@ -132,6 +131,72 @@ decl_event!(
     }
 );
 
+decl_error! {
+    pub enum Error {
+        /// Proposal cannot have an empty title"
+        EmptyTitleProvided,
+
+        /// Proposal cannot have an empty body
+        EmptyDescriptionProvided,
+
+        /// Title is too long
+        TitleIsTooLong,
+
+        /// Description is too long
+        DescriptionIsTooLong,
+
+        /// The proposal does not exist
+        ProposalNotFound,
+
+        /// Proposal is finalized already
+        ProposalFinalized,
+
+        /// The proposal have been already voted on
+        AlreadyVoted,
+
+        /// Not an author
+        NotAuthor,
+
+        /// Max active proposals number exceeded
+        MaxActiveProposalNumberExceeded,
+
+        /// Stake cannot be empty with this proposal
+        EmptyStake,
+
+        /// Stake should be empty for this proposal
+        StakeShouldBeEmpty,
+
+        /// Stake differs from the proposal requirements
+        StakeDiffersFromRequired,
+
+        /// Approval threshold cannot be zero
+        InvalidParameterApprovalThreshold,
+
+        /// Slashing threshold cannot be zero
+        InvalidParameterSlashingThreshold,
+
+        /// Require signed origin in extrinsics
+        RequireSignedOrigin,
+
+        /// Require root origin in extrinsics
+        RequireRootOrigin,
+    }
+}
+
+impl From<system::Error> for Error {
+    fn from(error: system::Error) -> Self {
+        match error {
+            system::Error::Other(msg) => Error::Other(msg),
+            system::Error::CannotLookup => Error::Other("CannotLookup"),
+            system::Error::BadSignature => Error::Other("BadSignature"),
+            system::Error::BlockFull => Error::Other("BlockFull"),
+            system::Error::RequireSignedOrigin => Error::RequireSignedOrigin,
+            system::Error::RequireRootOrigin => Error::RequireRootOrigin,
+            system::Error::RequireNoOrigin => Error::Other("RequireNoOrigin"),
+        }
+    }
+}
+
 // Storage for the proposals engine module
 decl_storage! {
     pub trait Store for Module<T: Trait> as ProposalEngine{
@@ -165,6 +230,8 @@ decl_storage! {
 decl_module! {
     /// 'Proposal engine' substrate module
     pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+        /// Predefined errors
+        type Error = Error;
 
         /// Emits an event. Default substrate implementation.
         fn deposit_event() = default;
@@ -176,17 +243,17 @@ decl_module! {
                 voter_id.clone(),
             )?;
 
-            ensure!(<Proposals<T>>::exists(proposal_id), errors::MSG_PROPOSAL_NOT_FOUND);
+            ensure!(<Proposals<T>>::exists(proposal_id), Error::ProposalNotFound);
             let mut proposal = Self::proposals(proposal_id);
 
-            ensure!(proposal.status == ProposalStatus::Active, errors::MSG_PROPOSAL_FINALIZED);
+            ensure!(proposal.status == ProposalStatus::Active, Error::ProposalFinalized);
 
             let did_not_vote_before = !<VoteExistsByProposalByVoter<T>>::exists(
                 proposal_id,
                 voter_id.clone(),
             );
 
-            ensure!(did_not_vote_before, errors::MSG_YOU_ALREADY_VOTED);
+            ensure!(did_not_vote_before, Error::AlreadyVoted);
 
             proposal.voting_results.add_vote(vote.clone());
 
@@ -204,11 +271,11 @@ decl_module! {
                 proposer_id.clone(),
             )?;
 
-            ensure!(<Proposals<T>>::exists(proposal_id), errors::MSG_PROPOSAL_NOT_FOUND);
+            ensure!(<Proposals<T>>::exists(proposal_id), Error::ProposalNotFound);
             let proposal = Self::proposals(proposal_id);
 
-            ensure!(proposer_id == proposal.proposer_id, errors::MSG_YOU_DONT_OWN_THIS_PROPOSAL);
-            ensure!(proposal.status == ProposalStatus::Active, errors::MSG_PROPOSAL_FINALIZED);
+            ensure!(proposer_id == proposal.proposer_id, Error::NotAuthor);
+            ensure!(proposal.status == ProposalStatus::Active, Error::ProposalFinalized);
 
             // mutation
 
@@ -219,10 +286,10 @@ decl_module! {
         pub fn veto_proposal(origin, proposal_id: T::ProposalId) {
             ensure_root(origin)?;
 
-            ensure!(<Proposals<T>>::exists(proposal_id), errors::MSG_PROPOSAL_NOT_FOUND);
+            ensure!(<Proposals<T>>::exists(proposal_id), Error::ProposalNotFound);
             let proposal = Self::proposals(proposal_id);
 
-            ensure!(proposal.status == ProposalStatus::Active, errors::MSG_PROPOSAL_FINALIZED);
+            ensure!(proposal.status == ProposalStatus::Active, Error::ProposalFinalized);
 
             // mutation
 
@@ -512,34 +579,34 @@ impl<T: Trait> Module<T> {
     fn ensure_create_proposal_parameters_are_valid(
         parameters: &ProposalParameters<T::BlockNumber, types::BalanceOf<T>>,
         title: &[u8],
-        body: &[u8],
+        description: &[u8],
         stake_balance: Option<types::BalanceOf<T>>,
-    ) -> dispatch::Result {
-        ensure!(!title.is_empty(), errors::MSG_EMPTY_TITLE_PROVIDED);
+    ) -> DispatchResult<Error> {
+        ensure!(!title.is_empty(), Error::EmptyTitleProvided);
         ensure!(
             title.len() as u32 <= T::TitleMaxLength::get(),
-            errors::MSG_TOO_LONG_TITLE
+            Error::TitleIsTooLong
         );
 
-        ensure!(!body.is_empty(), errors::MSG_EMPTY_BODY_PROVIDED);
+        ensure!(!description.is_empty(), Error::EmptyDescriptionProvided);
         ensure!(
-            body.len() as u32 <= T::DescriptionMaxLength::get(),
-            errors::MSG_TOO_LONG_BODY
+            description.len() as u32 <= T::DescriptionMaxLength::get(),
+            Error::DescriptionIsTooLong
         );
 
         ensure!(
             (Self::active_proposal_count()) < T::MaxActiveProposalLimit::get(),
-            errors::MSG_MAX_ACTIVE_PROPOSAL_NUMBER_EXCEEDED
+            Error::MaxActiveProposalNumberExceeded
         );
 
         ensure!(
             parameters.approval_threshold_percentage > 0,
-            errors::MSG_INVALID_PARAMETER_APPROVAL_THRESHOLD
+            Error::InvalidParameterApprovalThreshold
         );
 
         ensure!(
             parameters.slashing_threshold_percentage > 0,
-            errors::MSG_INVALID_PARAMETER_SLASHING_THRESHOLD
+            Error::InvalidParameterSlashingThreshold
         );
 
         // check stake parameters
@@ -547,15 +614,15 @@ impl<T: Trait> Module<T> {
             if let Some(staked_balance) = stake_balance {
                 ensure!(
                     required_stake == staked_balance,
-                    errors::MSG_STAKE_DIFFERS_FROM_REQUIRED
+                    Error::StakeDiffersFromRequired
                 );
             } else {
-                return Err(errors::MSG_STAKE_IS_EMPTY);
+                return Err(Error::EmptyStake);
             }
         }
 
         if stake_balance.is_some() && parameters.required_stake.is_none() {
-            return Err(errors::MSG_STAKE_SHOULD_BE_EMPTY);
+            return Err(Error::StakeShouldBeEmpty);
         }
 
         Ok(())

runtime-modules/proposals/engine/src/tests/mock/mod.rs

@@ -14,7 +14,7 @@ pub use sr_primitives::{
     weights::Weight,
     BuildStorage, DispatchError, Perbill,
 };
-use srml_support::{impl_outer_dispatch, impl_outer_event, impl_outer_origin, parameter_types};
+use srml_support::{impl_outer_event, impl_outer_origin, parameter_types};
 pub use system;
 
 mod balance_manager;

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

@@ -2,16 +2,15 @@
 
 use rstd::prelude::*;
 use rstd::vec::Vec;
-use sr_primitives::DispatchError;
 use srml_support::decl_module;
 pub trait Trait: system::Trait {}
 
 decl_module! {
     pub struct Module<T: Trait> for enum Call where origin: T::Origin {
-    	/// Working extrinsic test
+        /// Working extrinsic test
         pub fn dummy_proposal(_origin, _title: Vec<u8>, _description: Vec<u8>) {}
 
-		/// Broken extrinsic test
+        /// Broken extrinsic test
         pub fn faulty_proposal(_origin, _title: Vec<u8>, _description: Vec<u8>,) {
              Err("ExecutionFailed")?
         }

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

@@ -5,8 +5,8 @@ use mock::*;
 
 use codec::Encode;
 use rstd::rc::Rc;
-use sr_primitives::traits::{OnFinalize, OnInitialize};
-use srml_support::{dispatch, StorageMap, StorageValue};
+use sr_primitives::traits::{DispatchResult, OnFinalize, OnInitialize};
+use srml_support::{StorageMap, StorageValue};
 use system::RawOrigin;
 use system::{EventRecord, Phase};
 
@@ -70,10 +70,8 @@ impl Default for DummyProposalFixture {
     fn default() -> Self {
         let title = b"title".to_vec();
         let description = b"description".to_vec();
-        let dummy_proposal = mock::proposals::Call::<Test>::dummy_proposal(
-            title.clone(),
-            description.clone(),
-        );
+        let dummy_proposal =
+            mock::proposals::Call::<Test>::dummy_proposal(title.clone(), description.clone());
 
         DummyProposalFixture {
             parameters: ProposalParameters {
@@ -168,7 +166,7 @@ impl CancelProposalFixture {
         }
     }
 
-    fn cancel_and_assert(self, expected_result: dispatch::Result) {
+    fn cancel_and_assert(self, expected_result: DispatchResult<Error>) {
         assert_eq!(
             ProposalsEngine::cancel_proposal(
                 self.origin.into(),
@@ -196,7 +194,7 @@ impl VetoProposalFixture {
         VetoProposalFixture { origin, ..self }
     }
 
-    fn veto_and_assert(self, expected_result: dispatch::Result) {
+    fn veto_and_assert(self, expected_result: DispatchResult<Error>) {
         assert_eq!(
             ProposalsEngine::veto_proposal(self.origin.into(), self.proposal_id,),
             expected_result
@@ -224,11 +222,11 @@ impl VoteGenerator {
         self.vote_and_assert(vote_kind, Ok(()));
     }
 
-    fn vote_and_assert(&mut self, vote_kind: VoteKind, expected_result: dispatch::Result) {
+    fn vote_and_assert(&mut self, vote_kind: VoteKind, expected_result: DispatchResult<Error>) {
         assert_eq!(self.vote(vote_kind.clone()), expected_result);
     }
 
-    fn vote(&mut self, vote_kind: VoteKind) -> dispatch::Result {
+    fn vote(&mut self, vote_kind: VoteKind) -> DispatchResult<Error> {
         if self.auto_increment_voter_id {
             self.current_account_id += 1;
             self.current_voter_id += 1;
@@ -289,7 +287,7 @@ fn create_dummy_proposal_succeeds() {
 fn create_dummy_proposal_fails_with_insufficient_rights() {
     initial_test_ext().execute_with(|| {
         let dummy_proposal = DummyProposalFixture::default().with_origin(RawOrigin::None);
-        dummy_proposal.create_proposal_and_assert(Err("RequireSignedOrigin"));
+        dummy_proposal.create_proposal_and_assert(Err(Error::RequireSignedOrigin.into()));
     });
 }
 
@@ -309,7 +307,7 @@ fn vote_fails_with_insufficient_rights() {
     initial_test_ext().execute_with(|| {
         assert_eq!(
             ProposalsEngine::vote(system::RawOrigin::None.into(), 1, 1, VoteKind::Approve),
-            Err("RequireSignedOrigin")
+            Err(Error::Other("RequireSignedOrigin"))
         );
     });
 }
@@ -487,21 +485,21 @@ fn create_proposal_fails_with_invalid_body_or_title() {
     initial_test_ext().execute_with(|| {
         let mut dummy_proposal =
             DummyProposalFixture::default().with_title_and_body(Vec::new(), b"body".to_vec());
-        dummy_proposal.create_proposal_and_assert(Err("Proposal cannot have an empty title"));
+        dummy_proposal.create_proposal_and_assert(Err(Error::EmptyTitleProvided.into()));
 
         dummy_proposal =
             DummyProposalFixture::default().with_title_and_body(b"title".to_vec(), Vec::new());
-        dummy_proposal.create_proposal_and_assert(Err("Proposal cannot have an empty body"));
+        dummy_proposal.create_proposal_and_assert(Err(Error::EmptyDescriptionProvided.into()));
 
         let too_long_title = vec![0; 200];
         dummy_proposal =
             DummyProposalFixture::default().with_title_and_body(too_long_title, b"body".to_vec());
-        dummy_proposal.create_proposal_and_assert(Err("Title is too long"));
+        dummy_proposal.create_proposal_and_assert(Err(Error::TitleIsTooLong.into()));
 
         let too_long_body = vec![0; 11000];
         dummy_proposal =
             DummyProposalFixture::default().with_title_and_body(b"title".to_vec(), too_long_body);
-        dummy_proposal.create_proposal_and_assert(Err("Body is too long"));
+        dummy_proposal.create_proposal_and_assert(Err(Error::DescriptionIsTooLong.into()));
     });
 }
 
@@ -514,7 +512,7 @@ fn vote_fails_with_expired_voting_period() {
         run_to_block_and_finalize(6);
 
         let mut vote_generator = VoteGenerator::new(proposal_id);
-        vote_generator.vote_and_assert(VoteKind::Approve, Err("Proposal is finalized already"));
+        vote_generator.vote_and_assert(VoteKind::Approve, Err(Error::ProposalFinalized));
     });
 }
 
@@ -534,7 +532,7 @@ fn vote_fails_with_not_active_proposal() {
 
         let mut vote_generator_to_fail = VoteGenerator::new(proposal_id);
         vote_generator_to_fail
-            .vote_and_assert(VoteKind::Approve, Err("Proposal is finalized already"));
+            .vote_and_assert(VoteKind::Approve, Err(Error::ProposalFinalized));
     });
 }
 
@@ -542,7 +540,7 @@ fn vote_fails_with_not_active_proposal() {
 fn vote_fails_with_absent_proposal() {
     initial_test_ext().execute_with(|| {
         let mut vote_generator = VoteGenerator::new(2);
-        vote_generator.vote_and_assert(VoteKind::Approve, Err("This proposal does not exist"));
+        vote_generator.vote_and_assert(VoteKind::Approve, Err(Error::ProposalNotFound));
     });
 }
 
@@ -558,7 +556,7 @@ fn vote_fails_on_double_voting() {
         vote_generator.vote_and_assert_ok(VoteKind::Approve);
         vote_generator.vote_and_assert(
             VoteKind::Approve,
-            Err("You have already voted on this proposal"),
+            Err(Error::AlreadyVoted),
         );
     });
 }
@@ -601,7 +599,7 @@ fn cancel_proposal_fails_with_not_active_proposal() {
         run_to_block_and_finalize(6);
 
         let cancel_proposal = CancelProposalFixture::new(proposal_id);
-        cancel_proposal.cancel_and_assert(Err("Proposal is finalized already"));
+        cancel_proposal.cancel_and_assert(Err(Error::ProposalFinalized));
     });
 }
 
@@ -609,7 +607,7 @@ fn cancel_proposal_fails_with_not_active_proposal() {
 fn cancel_proposal_fails_with_not_existing_proposal() {
     initial_test_ext().execute_with(|| {
         let cancel_proposal = CancelProposalFixture::new(2);
-        cancel_proposal.cancel_and_assert(Err("This proposal does not exist"));
+        cancel_proposal.cancel_and_assert(Err(Error::ProposalNotFound));
     });
 }
 
@@ -622,7 +620,7 @@ fn cancel_proposal_fails_with_insufficient_rights() {
         let cancel_proposal = CancelProposalFixture::new(proposal_id)
             .with_origin(RawOrigin::Signed(2))
             .with_proposer(2);
-        cancel_proposal.cancel_and_assert(Err("You do not own this proposal"));
+        cancel_proposal.cancel_and_assert(Err(Error::NotAuthor));
     });
 }
 
@@ -673,7 +671,7 @@ fn veto_proposal_fails_with_not_active_proposal() {
         run_to_block_and_finalize(6);
 
         let veto_proposal = VetoProposalFixture::new(proposal_id);
-        veto_proposal.veto_and_assert(Err("Proposal is finalized already"));
+        veto_proposal.veto_and_assert(Err(Error::ProposalFinalized));
     });
 }
 
@@ -681,7 +679,7 @@ fn veto_proposal_fails_with_not_active_proposal() {
 fn veto_proposal_fails_with_not_existing_proposal() {
     initial_test_ext().execute_with(|| {
         let veto_proposal = VetoProposalFixture::new(2);
-        veto_proposal.veto_and_assert(Err("This proposal does not exist"));
+        veto_proposal.veto_and_assert(Err(Error::ProposalNotFound));
     });
 }
 
@@ -692,7 +690,7 @@ fn veto_proposal_fails_with_insufficient_rights() {
         let proposal_id = dummy_proposal.create_proposal_and_assert(Ok(1)).unwrap();
 
         let veto_proposal = VetoProposalFixture::new(proposal_id).with_origin(RawOrigin::Signed(2));
-        veto_proposal.veto_and_assert(Err("RequireRootOrigin"));
+        veto_proposal.veto_and_assert(Err(Error::RequireRootOrigin));
     });
 }
 
@@ -905,7 +903,7 @@ fn create_proposal_fails_on_exceeding_max_active_proposals_count() {
         }
 
         let dummy_proposal = DummyProposalFixture::default();
-        dummy_proposal.create_proposal_and_assert(Err("Max active proposals number exceeded"));
+        dummy_proposal.create_proposal_and_assert(Err(Error::MaxActiveProposalNumberExceeded.into()));
         // internal active proposal counter check
         assert_eq!(<ActiveProposalCount>::get(), 100);
     });
@@ -1007,13 +1005,13 @@ fn create_proposal_fais_with_invalid_stake_parameters() {
             .with_parameters(parameters_fixture.params())
             .with_stake(200);
 
-        dummy_proposal.create_proposal_and_assert(Err("Stake should be empty for this proposal"));
+        dummy_proposal.create_proposal_and_assert(Err(Error::StakeShouldBeEmpty.into()));
 
         let parameters_fixture_stake_200 = parameters_fixture.with_required_stake(200);
         dummy_proposal =
             DummyProposalFixture::default().with_parameters(parameters_fixture_stake_200.params());
 
-        dummy_proposal.create_proposal_and_assert(Err("Stake cannot be empty with this proposal"));
+        dummy_proposal.create_proposal_and_assert(Err(Error::EmptyStake.into()));
 
         let parameters_fixture_stake_300 = parameters_fixture.with_required_stake(300);
         dummy_proposal = DummyProposalFixture::default()
@@ -1021,7 +1019,7 @@ fn create_proposal_fais_with_invalid_stake_parameters() {
             .with_stake(200);
 
         dummy_proposal
-            .create_proposal_and_assert(Err("Stake differs from the proposal requirements"));
+            .create_proposal_and_assert(Err(Error::StakeDiffersFromRequired.into()));
     });
 }
 /* TODO: restore after the https://github.com/Joystream/substrate-runtime-joystream/issues/161