Migrate proposals to decl_error! macro

- migrate discussions
- create codex errors conversion helpers
- update tests

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

@@ -23,7 +23,7 @@ use rstd::vec::Vec;
 use srml_support::{decl_error, decl_module, decl_storage, ensure, print};
 use system::{ensure_root, RawOrigin};
 
-use proposal_engine::*;
+use proposal_engine::{ProposalParameters};
 
 /// 'Proposals codex' substrate module Trait
 pub trait Trait:
@@ -63,6 +63,9 @@ decl_error! {
 
         /// Require root origin in extrinsics
         RequireRootOrigin,
+
+        /// Errors from the proposal engine
+        ProposalsEngineError
     }
 }
 
@@ -70,16 +73,32 @@ 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::Other("RequireSignedOrigin"),
             system::Error::RequireRootOrigin => Error::RequireRootOrigin,
-            system::Error::RequireNoOrigin => Error::Other("RequireNoOrigin"),
+            _ => Error::Other(error.into()),
+        }
+    }
+}
+
+impl From<proposal_engine::Error> for Error {
+    fn from(error: proposal_engine::Error) -> Self {
+        match error {
+            proposal_engine::Error::Other(msg) => Error::Other(msg),
+            proposal_engine::Error::RequireRootOrigin => Error::RequireRootOrigin,
+            _ => Error::Other(error.into()),
         }
     }
 }
 
+impl From<proposal_discussion::Error> for Error {
+	fn from(error: proposal_discussion::Error) -> Self {
+        match error {
+            proposal_discussion::Error::Other(msg) => Error::Other(msg),
+            proposal_discussion::Error::RequireRootOrigin => Error::RequireRootOrigin,
+            _ => Error::Other(error.into()),
+        }
+	}
+}
+
 // Storage for the proposals codex module
 decl_storage! {
     pub trait Store for Module<T: Trait> as ProposalCodex{

runtime-modules/proposals/discussion/Cargo.toml

@@ -12,7 +12,7 @@ std = [
     'rstd/std',
     'srml-support/std',
     'primitives/std',
-    'runtime-primitives/std',
+    'sr-primitives/std',
     'system/std',
     'timestamp/std',
     'serde',
@@ -20,7 +20,6 @@ std = [
     'common/std',
 ]
 
-
 [dependencies.num_enum]
 default_features = false
 version = "0.4.2"
@@ -48,7 +47,7 @@ git = 'https://github.com/paritytech/substrate.git'
 package = 'sr-std'
 rev = 'c37bb08535c49a12320af7facfd555ce05cce2e8'
 
-[dependencies.runtime-primitives]
+[dependencies.sr-primitives]
 default_features = false
 git = 'https://github.com/paritytech/substrate.git'
 package = 'sr-primitives'

runtime-modules/proposals/discussion/src/errors.rs

@@ -1,10 +0,0 @@
-pub(crate) const MSG_NOT_AUTHOR: &str = "Author should match the post creator";
-pub(crate) const MSG_POST_EDITION_NUMBER_EXCEEDED: &str = "Post edition limit reached.";
-pub(crate) const MSG_EMPTY_TITLE_PROVIDED: &str = "Discussion cannot have an empty title";
-pub(crate) const MSG_TOO_LONG_TITLE: &str = "Title is too long";
-pub(crate) const MSG_THREAD_DOESNT_EXIST: &str = "Thread doesn't exist";
-pub(crate) const MSG_POST_DOESNT_EXIST: &str = "Post doesn't exist";
-pub(crate) const MSG_EMPTY_POST_PROVIDED: &str = "Post cannot be empty";
-pub(crate) const MSG_TOO_LONG_POST: &str = "Post is too long";
-pub(crate) const MSG_MAX_THREAD_IN_A_ROW_LIMIT_EXCEEDED: &str =
-    "Max number of threads by same author in a row limit exceeded";

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

@@ -15,7 +15,6 @@
 // Do not delete! Cannot be uncommented by default, because of Parity decl_module! issue.
 //#![warn(missing_docs)]
 
-mod errors;
 #[cfg(test)]
 mod tests;
 mod types;
@@ -23,16 +22,14 @@ mod types;
 use rstd::clone::Clone;
 use rstd::prelude::*;
 use rstd::vec::Vec;
-use srml_support::{decl_event, decl_module, decl_storage, ensure, Parameter};
+use srml_support::{decl_event, decl_module, decl_error, decl_storage, ensure, Parameter};
 
-use srml_support::traits::Get;
+use srml_support::traits::{Get};
 use types::{Post, Thread, ThreadCounter};
 
 use common::origin_validator::ActorOriginValidator;
 use membership::origin_validator::MemberId;
 
-// TODO: move errors to decl_error macro (after substrate version upgrade)
-
 decl_event!(
     /// Proposals engine events
     pub enum Event<T>
@@ -90,6 +87,53 @@ pub trait Trait: system::Trait + membership::members::Trait {
     type MaxThreadInARowNumber: Get<u32>;
 }
 
+decl_error! {
+    pub enum Error {
+        /// The size of the provided text for text proposal exceeded the limit
+        TextProposalSizeExceeded,
+
+        /// Author should match the post creator
+        NotAuthor,
+
+        ///  Post edition limit reached
+        PostEditionNumberExceeded,
+
+        /// Discussion cannot have an empty title
+        EmptyTitleProvided,
+
+        /// Title is too long
+        TitleIsTooLong,
+
+        /// Thread doesn't exist
+        ThreadDoesntExist,
+
+        /// Post doesn't exist
+        PostDoesntExist,
+
+        /// Post cannot be empty
+        EmptyPostProvided,
+
+        /// Post is too long
+        PostIsTooLong,
+
+        /// Max number of threads by same author in a row limit exceeded
+        MaxThreadInARowLimitExceeded,
+
+        /// 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::RequireRootOrigin => Error::RequireRootOrigin,
+            _ => Error::Other(error.into()),
+        }
+    }
+}
+
 // Storage for the proposals discussion module
 decl_storage! {
     pub trait Store for Module<T: Trait> as ProposalDiscussion {
@@ -116,6 +160,8 @@ decl_storage! {
 decl_module! {
     /// 'Proposal discussion' 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;
@@ -131,12 +177,12 @@ decl_module! {
                 origin,
                 post_author_id.clone(),
             )?;
-            ensure!(<ThreadById<T>>::exists(thread_id), errors::MSG_THREAD_DOESNT_EXIST);
+            ensure!(<ThreadById<T>>::exists(thread_id), Error::ThreadDoesntExist);
 
-            ensure!(!text.is_empty(), errors::MSG_EMPTY_POST_PROVIDED);
+            ensure!(!text.is_empty(),Error::EmptyPostProvided);
             ensure!(
                 text.len() as u32 <= T::PostLengthLimit::get(),
-                errors::MSG_TOO_LONG_POST
+                Error::PostIsTooLong
             );
 
             // mutation
@@ -172,20 +218,20 @@ decl_module! {
                 post_author_id.clone(),
             )?;
 
-            ensure!(<ThreadById<T>>::exists(thread_id), errors::MSG_THREAD_DOESNT_EXIST);
-            ensure!(<PostThreadIdByPostId<T>>::exists(thread_id, post_id), errors::MSG_POST_DOESNT_EXIST);
+            ensure!(<ThreadById<T>>::exists(thread_id), Error::ThreadDoesntExist);
+            ensure!(<PostThreadIdByPostId<T>>::exists(thread_id, post_id), Error::PostDoesntExist);
 
-            ensure!(!text.is_empty(), errors::MSG_EMPTY_POST_PROVIDED);
+            ensure!(!text.is_empty(), Error::EmptyPostProvided);
             ensure!(
                 text.len() as u32 <= T::PostLengthLimit::get(),
-                errors::MSG_TOO_LONG_POST
+                Error::PostIsTooLong
             );
 
             let post = <PostThreadIdByPostId<T>>::get(&thread_id, &post_id);
 
-            ensure!(post.author_id == post_author_id, errors::MSG_NOT_AUTHOR);
+            ensure!(post.author_id == post_author_id, Error::NotAuthor);
             ensure!(post.edition_number < T::MaxPostEditionNumber::get(),
-                errors::MSG_POST_EDITION_NUMBER_EXCEEDED);
+                Error::PostEditionNumberExceeded);
 
             let new_post = Post {
                 text,
@@ -214,13 +260,13 @@ impl<T: Trait> Module<T> {
         origin: T::Origin,
         thread_author_id: MemberId<T>,
         title: Vec<u8>,
-    ) -> Result<T::ThreadId, &'static str> {
+    ) -> Result<T::ThreadId, Error> {
         T::ThreadAuthorOriginValidator::ensure_actor_origin(origin, thread_author_id.clone())?;
 
-        ensure!(!title.is_empty(), errors::MSG_EMPTY_TITLE_PROVIDED);
+        ensure!(!title.is_empty(), Error::EmptyTitleProvided);
         ensure!(
             title.len() as u32 <= T::ThreadTitleLengthLimit::get(),
-            errors::MSG_TOO_LONG_TITLE
+            Error::TitleIsTooLong
         );
 
         // get new 'threads in a row' counter for the author
@@ -228,7 +274,7 @@ impl<T: Trait> Module<T> {
 
         ensure!(
             current_thread_counter.counter as u32 <= T::MaxThreadInARowNumber::get(),
-            errors::MSG_MAX_THREAD_IN_A_ROW_LIMIT_EXCEEDED
+            Error::MaxThreadInARowLimitExceeded
         );
 
         let next_thread_count_value = Self::thread_count() + 1;

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

@@ -3,7 +3,7 @@
 pub use system;
 
 pub use primitives::{Blake2Hasher, H256};
-pub use runtime_primitives::{
+pub use sr_primitives::{
     testing::{Digest, DigestItem, Header, UintAuthorityId},
     traits::{BlakeTwo256, Convert, IdentityLookup, OnFinalize},
     weights::Weight,

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

@@ -2,7 +2,6 @@ mod mock;
 
 use mock::*;
 
-use crate::errors::*;
 use crate::*;
 use system::RawOrigin;
 use system::{EventRecord, Phase};
@@ -93,7 +92,7 @@ impl DiscussionFixture {
         }
     }
 
-    fn create_discussion_and_assert(&self, result: Result<u32, &'static str>) -> Option<u32> {
+    fn create_discussion_and_assert(&self, result: Result<u32, Error>) -> Option<u32> {
         let create_discussion_result = Discussions::create_thread(
             self.origin.clone().into(),
             self.author_id,
@@ -148,7 +147,7 @@ impl PostFixture {
         }
     }
 
-    fn add_post_and_assert(&mut self, result: Result<(), &'static str>) -> Option<u32> {
+    fn add_post_and_assert(&mut self, result: Result<(), Error>) -> Option<u32> {
         let add_post_result = Discussions::add_post(
             self.origin.clone().into(),
             self.author_id,
@@ -168,7 +167,7 @@ impl PostFixture {
     fn update_post_with_text_and_assert(
         &mut self,
         new_text: Vec<u8>,
-        result: Result<(), &'static str>,
+        result: Result<(), Error>,
     ) {
         let add_post_result = Discussions::update_post(
             self.origin.clone().into(),
@@ -181,7 +180,7 @@ impl PostFixture {
         assert_eq!(add_post_result, result);
     }
 
-    fn update_post_and_assert(&mut self, result: Result<(), &'static str>) {
+    fn update_post_and_assert(&mut self, result: Result<(), Error>) {
         self.update_post_with_text_and_assert(self.text.clone(), result);
     }
 }
@@ -249,7 +248,7 @@ fn update_post_call_failes_because_of_post_edition_limit() {
             post_fixture.update_post_and_assert(Ok(()));
         }
 
-        post_fixture.update_post_and_assert(Err(MSG_POST_EDITION_NUMBER_EXCEEDED));
+        post_fixture.update_post_and_assert(Err(Error::PostEditionNumberExceeded));
     });
 }
 
@@ -268,11 +267,11 @@ fn update_post_call_failes_because_of_the_wrong_author() {
 
         post_fixture = post_fixture.with_author(2);
 
-        post_fixture.update_post_and_assert(Err("Invalid author"));
+        post_fixture.update_post_and_assert(Err(Error::Other("Invalid author")));
 
         post_fixture = post_fixture.with_origin(RawOrigin::None).with_author(2);
 
-        post_fixture.update_post_and_assert(Err(MSG_NOT_AUTHOR));
+        post_fixture.update_post_and_assert(Err(Error::NotAuthor));
     });
 }
 
@@ -317,10 +316,10 @@ fn thread_content_check_succeeded() {
 fn create_discussion_call_with_bad_title_failed() {
     initial_test_ext().execute_with(|| {
         let mut discussion_fixture = DiscussionFixture::default().with_title(Vec::new());
-        discussion_fixture.create_discussion_and_assert(Err(MSG_EMPTY_TITLE_PROVIDED));
+        discussion_fixture.create_discussion_and_assert(Err(Error::EmptyTitleProvided));
 
         discussion_fixture = DiscussionFixture::default().with_title([0; 201].to_vec());
-        discussion_fixture.create_discussion_and_assert(Err(MSG_TOO_LONG_TITLE));
+        discussion_fixture.create_discussion_and_assert(Err(Error::TitleIsTooLong));
     });
 }
 
@@ -333,7 +332,7 @@ fn add_post_call_with_invalid_thread_failed() {
             .unwrap();
 
         let mut post_fixture = PostFixture::default_for_thread(2);
-        post_fixture.add_post_and_assert(Err(MSG_THREAD_DOESNT_EXIST));
+        post_fixture.add_post_and_assert(Err(Error::ThreadDoesntExist));
     });
 }
 
@@ -349,7 +348,7 @@ fn update_post_call_with_invalid_post_failed() {
         post_fixture1.add_post_and_assert(Ok(())).unwrap();
 
         let mut post_fixture2 = post_fixture1.change_post_id(2);
-        post_fixture2.update_post_and_assert(Err(MSG_POST_DOESNT_EXIST));
+        post_fixture2.update_post_and_assert(Err(Error::PostDoesntExist));
     });
 }
 
@@ -365,7 +364,7 @@ fn update_post_call_with_invalid_thread_failed() {
         post_fixture1.add_post_and_assert(Ok(())).unwrap();
 
         let mut post_fixture2 = post_fixture1.change_thread_id(2);
-        post_fixture2.update_post_and_assert(Err(MSG_THREAD_DOESNT_EXIST));
+        post_fixture2.update_post_and_assert(Err(Error::ThreadDoesntExist));
     });
 }
 
@@ -378,11 +377,11 @@ fn add_post_call_with_invalid_text_failed() {
             .unwrap();
 
         let mut post_fixture1 = PostFixture::default_for_thread(thread_id).with_text(Vec::new());
-        post_fixture1.add_post_and_assert(Err(MSG_EMPTY_POST_PROVIDED));
+        post_fixture1.add_post_and_assert(Err(Error::EmptyPostProvided));
 
         let mut post_fixture2 =
             PostFixture::default_for_thread(thread_id).with_text([0; 2001].to_vec());
-        post_fixture2.add_post_and_assert(Err(MSG_TOO_LONG_POST));
+        post_fixture2.add_post_and_assert(Err(Error::PostIsTooLong));
     });
 }
 
@@ -398,10 +397,10 @@ fn update_post_call_with_invalid_text_failed() {
         post_fixture1.add_post_and_assert(Ok(()));
 
         let mut post_fixture2 = post_fixture1.with_text(Vec::new());
-        post_fixture2.update_post_and_assert(Err(MSG_EMPTY_POST_PROVIDED));
+        post_fixture2.update_post_and_assert(Err(Error::EmptyPostProvided));
 
         let mut post_fixture3 = post_fixture2.with_text([0; 2001].to_vec());
-        post_fixture3.update_post_and_assert(Err(MSG_TOO_LONG_POST));
+        post_fixture3.update_post_and_assert(Err(Error::PostIsTooLong));
     });
 }
 
@@ -416,7 +415,7 @@ fn add_discussion_thread_fails_because_of_max_thread_by_same_author_in_a_row_lim
         }
 
         discussion_fixture
-            .create_discussion_and_assert(Err(MSG_MAX_THREAD_IN_A_ROW_LIMIT_EXCEEDED));
+            .create_discussion_and_assert(Err(Error::MaxThreadInARowLimitExceeded));
     });
 }
 
@@ -424,11 +423,11 @@ fn add_discussion_thread_fails_because_of_max_thread_by_same_author_in_a_row_lim
 fn add_discussion_thread_fails_because_of_invalid_author_origin() {
     initial_test_ext().execute_with(|| {
         let discussion_fixture = DiscussionFixture::default().with_author(2);
-        discussion_fixture.create_discussion_and_assert(Err("Invalid author"));
+        discussion_fixture.create_discussion_and_assert(Err(Error::Other("Invalid author")));
 
         let discussion_fixture = DiscussionFixture::default()
             .with_origin(RawOrigin::Signed(3))
             .with_author(2);
-        discussion_fixture.create_discussion_and_assert(Err("Invalid author"));
+        discussion_fixture.create_discussion_and_assert(Err(Error::Other("Invalid author")));
     });
 }

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

@@ -175,9 +175,6 @@ decl_error! {
         /// Slashing threshold cannot be zero
         InvalidParameterSlashingThreshold,
 
-        /// Require signed origin in extrinsics
-        RequireSignedOrigin,
-
         /// Require root origin in extrinsics
         RequireRootOrigin,
     }
@@ -187,12 +184,8 @@ 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"),
+            _ => Error::Other(error.into()),
         }
     }
 }
@@ -331,7 +324,7 @@ impl<T: Trait> Module<T> {
         description: Vec<u8>,
         stake_balance: Option<types::BalanceOf<T>>,
         proposal_code: Vec<u8>,
-    ) -> Result<T::ProposalId, &'static str> {
+    ) -> Result<T::ProposalId, Error> {
         let account_id =
             T::ProposerOriginValidator::ensure_actor_origin(origin, proposer_id.clone())?;
 

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

@@ -124,7 +124,7 @@ impl DummyProposalFixture {
         }
     }
 
-    fn create_proposal_and_assert(self, result: Result<u32, &'static str>) -> Option<u32> {
+    fn create_proposal_and_assert(self, result: Result<u32, Error>) -> Option<u32> {
         let proposal_id_result = ProposalsEngine::create_proposal(
             self.origin.into(),
             self.proposer_id,
@@ -287,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(Error::RequireSignedOrigin.into()));
+        dummy_proposal.create_proposal_and_assert(Err(Error::Other("RequireSignedOrigin")));
     });
 }
 
@@ -992,7 +992,7 @@ fn create_dummy_proposal_fail_with_stake_on_empty_account() {
             .with_origin(RawOrigin::Signed(account_id))
             .with_stake(required_stake);
 
-        dummy_proposal.create_proposal_and_assert(Err("too few free funds in account"));
+        dummy_proposal.create_proposal_and_assert(Err(Error::Other("too few free funds in account")));
     });
 }