Merge pull request #422 from mnaamani/use-common-thread-post-id-types

Use common thread post id types

node/src/forum_config/from_serialized.rs

@@ -1,7 +1,9 @@
+#![allow(clippy::type_complexity)]
+
 use super::new_validation;
 use node_runtime::{
-    forum::{Category, CategoryId, Post, PostId, Thread, ThreadId},
-    AccountId, BlockNumber, ForumConfig, Moment,
+    forum::{Category, CategoryId, Post, Thread},
+    AccountId, BlockNumber, ForumConfig, Moment, PostId, ThreadId,
 };
 use serde::Deserialize;
 use serde_json::Result;
@@ -9,8 +11,11 @@ use serde_json::Result;
 #[derive(Deserialize)]
 struct ForumData {
     categories: Vec<(CategoryId, Category<BlockNumber, Moment, AccountId>)>,
-    posts: Vec<(PostId, Post<BlockNumber, Moment, AccountId>)>,
-    threads: Vec<(ThreadId, Thread<BlockNumber, Moment, AccountId>)>,
+    posts: Vec<(
+        PostId,
+        Post<BlockNumber, Moment, AccountId, ThreadId, PostId>,
+    )>,
+    threads: Vec<(ThreadId, Thread<BlockNumber, Moment, AccountId, ThreadId>)>,
 }
 
 fn parse_forum_json() -> Result<ForumData> {

runtime-modules/forum/src/lib.rs

@@ -12,8 +12,9 @@ use serde_derive::{Deserialize, Serialize};
 use rstd::borrow::ToOwned;
 use rstd::prelude::*;
 
-use codec::{Decode, Encode};
-use srml_support::{decl_event, decl_module, decl_storage, dispatch, ensure};
+use codec::{Codec, Decode, Encode};
+use runtime_primitives::traits::{MaybeSerialize, Member, One, SimpleArithmetic};
+use srml_support::{decl_event, decl_module, decl_storage, dispatch, ensure, Parameter};
 
 mod mock;
 mod tests;
@@ -156,13 +157,10 @@ pub struct PostTextChange<BlockNumber, Moment> {
     text: Vec<u8>,
 }
 
-/// Represents a post identifier
-pub type PostId = u64;
-
 /// Represents a thread post
 #[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))]
 #[derive(Encode, Decode, Default, Clone, PartialEq, Eq)]
-pub struct Post<BlockNumber, Moment, AccountId> {
+pub struct Post<BlockNumber, Moment, AccountId, ThreadId, PostId> {
     /// Post identifier
     id: PostId,
 
@@ -192,13 +190,10 @@ pub struct Post<BlockNumber, Moment, AccountId> {
     author_id: AccountId,
 }
 
-/// Represents a thread identifier
-pub type ThreadId = u64;
-
 /// Represents a thread
 #[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))]
 #[derive(Encode, Decode, Default, Clone, PartialEq, Eq)]
-pub struct Thread<BlockNumber, Moment, AccountId> {
+pub struct Thread<BlockNumber, Moment, AccountId, ThreadId> {
     /// Thread identifier
     id: ThreadId,
 
@@ -238,7 +233,7 @@ pub struct Thread<BlockNumber, Moment, AccountId> {
     author_id: AccountId,
 }
 
-impl<BlockNumber, Moment, AccountId> Thread<BlockNumber, Moment, AccountId> {
+impl<BlockNumber, Moment, AccountId, ThreadId> Thread<BlockNumber, Moment, AccountId, ThreadId> {
     fn num_posts_ever_created(&self) -> u32 {
         self.num_unmoderated_posts + self.num_moderated_posts
     }
@@ -321,6 +316,26 @@ pub trait Trait: system::Trait + timestamp::Trait + Sized {
     type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>;
 
     type MembershipRegistry: ForumUserRegistry<Self::AccountId>;
+
+    /// Thread Id type
+    type ThreadId: Parameter
+        + Member
+        + SimpleArithmetic
+        + Codec
+        + Default
+        + Copy
+        + MaybeSerialize
+        + PartialEq;
+
+    /// Post Id type
+    type PostId: Parameter
+        + Member
+        + SimpleArithmetic
+        + Codec
+        + Default
+        + Copy
+        + MaybeSerialize
+        + PartialEq;
 }
 
 decl_storage! {
@@ -333,16 +348,16 @@ decl_storage! {
         pub NextCategoryId get(next_category_id) config(): CategoryId;
 
         /// Map thread identifier to corresponding thread.
-        pub ThreadById get(thread_by_id) config(): map ThreadId => Thread<T::BlockNumber, T::Moment, T::AccountId>;
+        pub ThreadById get(thread_by_id) config(): map T::ThreadId => Thread<T::BlockNumber, T::Moment, T::AccountId, T::ThreadId>;
 
         /// Thread identifier value to be used for next Thread in threadById.
-        pub NextThreadId get(next_thread_id) config(): ThreadId;
+        pub NextThreadId get(next_thread_id) config(): T::ThreadId;
 
         /// Map post identifier to corresponding post.
-        pub PostById get(post_by_id) config(): map PostId => Post<T::BlockNumber, T::Moment, T::AccountId>;
+        pub PostById get(post_by_id) config(): map T::PostId => Post<T::BlockNumber, T::Moment, T::AccountId, T::ThreadId, T::PostId>;
 
         /// Post identifier value to be used for for next post created.
-        pub NextPostId get(next_post_id) config(): PostId;
+        pub NextPostId get(next_post_id) config(): T::PostId;
 
         /// Account of forum sudo.
         pub ForumSudo get(forum_sudo) config(): Option<T::AccountId>;
@@ -386,6 +401,8 @@ decl_event!(
     pub enum Event<T>
     where
         <T as system::Trait>::AccountId,
+        <T as Trait>::ThreadId,
+        <T as Trait>::PostId,
     {
         /// A category was introduced
         CategoryCreated(CategoryId),
@@ -632,7 +649,7 @@ decl_module! {
         }
 
         /// Moderate thread
-        fn moderate_thread(origin, thread_id: ThreadId, rationale: Vec<u8>) -> dispatch::Result {
+        fn moderate_thread(origin, thread_id: T::ThreadId, rationale: Vec<u8>) -> dispatch::Result {
 
             // Check that its a valid signature
             let who = ensure_signed(origin)?;
@@ -683,7 +700,7 @@ decl_module! {
         }
 
         /// Edit post text
-        fn add_post(origin, thread_id: ThreadId, text: Vec<u8>) -> dispatch::Result {
+        fn add_post(origin, thread_id: T::ThreadId, text: Vec<u8>) -> dispatch::Result {
 
             /*
              * Update SPEC with new errors,
@@ -720,7 +737,7 @@ decl_module! {
         }
 
         /// Edit post text
-        fn edit_post_text(origin, post_id: PostId, new_text: Vec<u8>) -> dispatch::Result {
+        fn edit_post_text(origin, post_id: T::PostId, new_text: Vec<u8>) -> dispatch::Result {
 
             /* Edit spec.
               - forum member guard missing
@@ -767,7 +784,7 @@ decl_module! {
         }
 
         /// Moderate post
-        fn moderate_post(origin, post_id: PostId, rationale: Vec<u8>) -> dispatch::Result {
+        fn moderate_post(origin, post_id: T::PostId, rationale: Vec<u8>) -> dispatch::Result {
 
             // Check that its a valid signature
             let who = ensure_signed(origin)?;
@@ -867,8 +884,9 @@ impl<T: Trait> Module<T> {
     }
 
     fn ensure_post_is_mutable(
-        post_id: PostId,
-    ) -> Result<Post<T::BlockNumber, T::Moment, T::AccountId>, &'static str> {
+        post_id: T::PostId,
+    ) -> Result<Post<T::BlockNumber, T::Moment, T::AccountId, T::ThreadId, T::PostId>, &'static str>
+    {
         // Make sure post exists
         let post = Self::ensure_post_exists(post_id)?;
 
@@ -882,8 +900,9 @@ impl<T: Trait> Module<T> {
     }
 
     fn ensure_post_exists(
-        post_id: PostId,
-    ) -> Result<Post<T::BlockNumber, T::Moment, T::AccountId>, &'static str> {
+        post_id: T::PostId,
+    ) -> Result<Post<T::BlockNumber, T::Moment, T::AccountId, T::ThreadId, T::PostId>, &'static str>
+    {
         if <PostById<T>>::exists(post_id) {
             Ok(<PostById<T>>::get(post_id))
         } else {
@@ -892,8 +911,8 @@ impl<T: Trait> Module<T> {
     }
 
     fn ensure_thread_is_mutable(
-        thread_id: ThreadId,
-    ) -> Result<Thread<T::BlockNumber, T::Moment, T::AccountId>, &'static str> {
+        thread_id: T::ThreadId,
+    ) -> Result<Thread<T::BlockNumber, T::Moment, T::AccountId, T::ThreadId>, &'static str> {
         // Make sure thread exists
         let thread = Self::ensure_thread_exists(thread_id)?;
 
@@ -907,8 +926,8 @@ impl<T: Trait> Module<T> {
     }
 
     fn ensure_thread_exists(
-        thread_id: ThreadId,
-    ) -> Result<Thread<T::BlockNumber, T::Moment, T::AccountId>, &'static str> {
+        thread_id: T::ThreadId,
+    ) -> Result<Thread<T::BlockNumber, T::Moment, T::AccountId, T::ThreadId>, &'static str> {
         if <ThreadById<T>>::exists(thread_id) {
             Ok(<ThreadById<T>>::get(thread_id))
         } else {
@@ -1045,12 +1064,12 @@ impl<T: Trait> Module<T> {
         category_id: CategoryId,
         title: &[u8],
         author_id: &T::AccountId,
-    ) -> Thread<T::BlockNumber, T::Moment, T::AccountId> {
+    ) -> Thread<T::BlockNumber, T::Moment, T::AccountId, T::ThreadId> {
         // Get category
         let category = <CategoryById<T>>::get(category_id);
 
         // Create and add new thread
-        let new_thread_id = NextThreadId::get();
+        let new_thread_id = NextThreadId::<T>::get();
 
         let new_thread = Thread {
             id: new_thread_id,
@@ -1068,8 +1087,8 @@ impl<T: Trait> Module<T> {
         <ThreadById<T>>::insert(new_thread_id, new_thread.clone());
 
         // Update next thread id
-        NextThreadId::mutate(|n| {
-            *n += 1;
+        NextThreadId::<T>::mutate(|n| {
+            *n += One::one();
         });
 
         // Update unmoderated thread count in corresponding category
@@ -1083,15 +1102,15 @@ impl<T: Trait> Module<T> {
     /// Creates and ads a new post ot the given thread, and makes all required state updates
     /// `thread_id` must be valid
     fn add_new_post(
-        thread_id: ThreadId,
+        thread_id: T::ThreadId,
         text: &[u8],
         author_id: &T::AccountId,
-    ) -> Post<T::BlockNumber, T::Moment, T::AccountId> {
+    ) -> Post<T::BlockNumber, T::Moment, T::AccountId, T::ThreadId, T::PostId> {
         // Get thread
         let thread = <ThreadById<T>>::get(thread_id);
 
         // Make and add initial post
-        let new_post_id = NextPostId::get();
+        let new_post_id = NextPostId::<T>::get();
 
         let new_post = Post {
             id: new_post_id,
@@ -1108,8 +1127,8 @@ impl<T: Trait> Module<T> {
         <PostById<T>>::insert(new_post_id, new_post.clone());
 
         // Update next post id
-        NextPostId::mutate(|n| {
-            *n += 1;
+        NextPostId::<T>::mutate(|n| {
+            *n += One::one();
         });
 
         // Update unmoderated post count of thread

runtime-modules/forum/src/mock.rs

@@ -100,6 +100,8 @@ impl timestamp::Trait for Runtime {
 impl Trait for Runtime {
     type Event = ();
     type MembershipRegistry = registry::TestMembershipRegistryModule;
+    type ThreadId = u64;
+    type PostId = u64;
 }
 
 #[derive(Clone)]
@@ -123,9 +125,9 @@ pub const NOT_MEMBER_ORIGIN: OriginType = OriginType::Signed(222);
 
 pub const INVLAID_CATEGORY_ID: CategoryId = 333;
 
-pub const INVLAID_THREAD_ID: ThreadId = 444;
+pub const INVLAID_THREAD_ID: RuntimeThreadId = 444;
 
-pub const INVLAID_POST_ID: ThreadId = 555;
+pub const INVLAID_POST_ID: RuntimePostId = 555;
 
 pub fn generate_text(len: usize) -> Vec<u8> {
     vec![b'x'; len]
@@ -228,7 +230,7 @@ impl CreateThreadFixture {
 
 pub struct CreatePostFixture {
     pub origin: OriginType,
-    pub thread_id: ThreadId,
+    pub thread_id: RuntimeThreadId,
     pub text: Vec<u8>,
     pub result: dispatch::Result,
 }
@@ -285,7 +287,7 @@ pub fn assert_create_thread(
 
 pub fn assert_create_post(
     forum_sudo: OriginType,
-    thread_id: ThreadId,
+    thread_id: RuntimeThreadId,
     expected_result: dispatch::Result,
 ) {
     CreatePostFixture {
@@ -312,7 +314,7 @@ pub fn create_root_category(forum_sudo: OriginType) -> CategoryId {
 
 pub fn create_root_category_and_thread(
     forum_sudo: OriginType,
-) -> (OriginType, CategoryId, ThreadId) {
+) -> (OriginType, CategoryId, RuntimeThreadId) {
     let member_origin = create_forum_member();
     let category_id = create_root_category(forum_sudo);
     let thread_id = TestForumModule::next_thread_id();
@@ -331,7 +333,7 @@ pub fn create_root_category_and_thread(
 
 pub fn create_root_category_and_thread_and_post(
     forum_sudo: OriginType,
-) -> (OriginType, CategoryId, ThreadId, PostId) {
+) -> (OriginType, CategoryId, RuntimeThreadId, RuntimePostId) {
     let (member_origin, category_id, thread_id) = create_root_category_and_thread(forum_sudo);
     let post_id = TestForumModule::next_post_id();
 
@@ -348,7 +350,7 @@ pub fn create_root_category_and_thread_and_post(
 
 pub fn moderate_thread(
     forum_sudo: OriginType,
-    thread_id: ThreadId,
+    thread_id: RuntimeThreadId,
     rationale: Vec<u8>,
 ) -> dispatch::Result {
     TestForumModule::moderate_thread(mock_origin(forum_sudo), thread_id, rationale)
@@ -356,7 +358,7 @@ pub fn moderate_thread(
 
 pub fn moderate_post(
     forum_sudo: OriginType,
-    post_id: PostId,
+    post_id: RuntimePostId,
     rationale: Vec<u8>,
 ) -> dispatch::Result {
     TestForumModule::moderate_post(mock_origin(forum_sudo), post_id, rationale)
@@ -456,23 +458,28 @@ pub type RuntimeThread = Thread<
     <Runtime as system::Trait>::BlockNumber,
     <Runtime as timestamp::Trait>::Moment,
     <Runtime as system::Trait>::AccountId,
+    RuntimeThreadId,
 >;
 pub type RuntimePost = Post<
     <Runtime as system::Trait>::BlockNumber,
     <Runtime as timestamp::Trait>::Moment,
     <Runtime as system::Trait>::AccountId,
+    RuntimeThreadId,
+    RuntimePostId,
 >;
 pub type RuntimeBlockchainTimestamp = BlockchainTimestamp<
     <Runtime as system::Trait>::BlockNumber,
     <Runtime as timestamp::Trait>::Moment,
 >;
+pub type RuntimeThreadId = <Runtime as Trait>::ThreadId;
+pub type RuntimePostId = <Runtime as Trait>::PostId;
 
 pub fn genesis_config(
     category_by_id: &RuntimeMap<CategoryId, RuntimeCategory>,
     next_category_id: u64,
-    thread_by_id: &RuntimeMap<ThreadId, RuntimeThread>,
+    thread_by_id: &RuntimeMap<RuntimeThreadId, RuntimeThread>,
     next_thread_id: u64,
-    post_by_id: &RuntimeMap<PostId, RuntimePost>,
+    post_by_id: &RuntimeMap<RuntimePostId, RuntimePost>,
     next_post_id: u64,
     forum_sudo: <Runtime as system::Trait>::AccountId,
     category_title_constraint: &InputValidationLengthConstraint,

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

@@ -141,8 +141,8 @@ parameter_types! {
 impl proposal_discussion::Trait for Test {
     type Event = ();
     type PostAuthorOriginValidator = ();
-    type ThreadId = u32;
-    type PostId = u32;
+    type ThreadId = u64;
+    type PostId = u64;
     type MaxPostEditionNumber = MaxPostEditionNumber;
     type ThreadTitleLengthLimit = ThreadTitleLengthLimit;
     type PostLengthLimit = PostLengthLimit;

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

@@ -95,10 +95,10 @@ pub trait Trait: system::Trait + membership::members::Trait {
     >;
 
     /// Discussion thread Id type
-    type ThreadId: From<u32> + Into<u32> + Parameter + Default + Copy;
+    type ThreadId: From<u64> + Into<u64> + Parameter + Default + Copy;
 
     /// Post Id type
-    type PostId: From<u32> + Parameter + Default + Copy;
+    type PostId: From<u64> + Parameter + Default + Copy;
 
     /// Defines post edition number limit.
     type MaxPostEditionNumber: Get<u32>;
@@ -166,14 +166,14 @@ decl_storage! {
             Thread<MemberId<T>, T::BlockNumber>;
 
         /// Count of all threads that have been created.
-        pub ThreadCount get(fn thread_count): u32;
+        pub ThreadCount get(fn thread_count): u64;
 
         /// Map thread id and post id to corresponding post.
         pub PostThreadIdByPostId: double_map T::ThreadId, twox_128(T::PostId) =>
              Post<MemberId<T>, T::BlockNumber, T::ThreadId>;
 
         /// Count of all posts that have been created.
-        pub PostCount get(fn post_count): u32;
+        pub PostCount get(fn post_count): u64;
 
         /// Last author thread counter (part of the antispam mechanism)
         pub LastThreadAuthorCounter get(fn last_thread_author_counter):

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

@@ -87,8 +87,8 @@ impl membership::members::Trait for Test {
 impl crate::Trait for Test {
     type Event = TestEvent;
     type PostAuthorOriginValidator = ();
-    type ThreadId = u32;
-    type PostId = u32;
+    type ThreadId = u64;
+    type PostId = u64;
     type MaxPostEditionNumber = MaxPostEditionNumber;
     type ThreadTitleLengthLimit = ThreadTitleLengthLimit;
     type PostLengthLimit = PostLengthLimit;

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

@@ -8,7 +8,7 @@ use system::{EventRecord, Phase};
 
 struct EventFixture;
 impl EventFixture {
-    fn assert_events(expected_raw_events: Vec<RawEvent<u32, u64, u32>>) {
+    fn assert_events(expected_raw_events: Vec<RawEvent<u64, u64, u64>>) {
         let expected_events = expected_raw_events
             .iter()
             .map(|ev| EventRecord {
@@ -23,13 +23,13 @@ impl EventFixture {
 }
 
 struct TestPostEntry {
-    pub post_id: u32,
+    pub post_id: u64,
     pub text: Vec<u8>,
     pub edition_number: u32,
 }
 
 struct TestThreadEntry {
-    pub thread_id: u32,
+    pub thread_id: u64,
     pub title: Vec<u8>,
 }
 
@@ -81,7 +81,7 @@ impl DiscussionFixture {
         DiscussionFixture { title, ..self }
     }
 
-    fn create_discussion_and_assert(&self, result: Result<u32, Error>) -> Option<u32> {
+    fn create_discussion_and_assert(&self, result: Result<u64, Error>) -> Option<u64> {
         let create_discussion_result =
             Discussions::create_thread(self.author_id, self.title.clone());
 
@@ -94,13 +94,13 @@ impl DiscussionFixture {
 struct PostFixture {
     pub text: Vec<u8>,
     pub origin: RawOrigin<u64>,
-    pub thread_id: u32,
-    pub post_id: Option<u32>,
+    pub thread_id: u64,
+    pub post_id: Option<u64>,
     pub author_id: u64,
 }
 
 impl PostFixture {
-    fn default_for_thread(thread_id: u32) -> Self {
+    fn default_for_thread(thread_id: u64) -> Self {
         PostFixture {
             text: b"text".to_vec(),
             author_id: 1,
@@ -122,18 +122,18 @@ impl PostFixture {
         PostFixture { author_id, ..self }
     }
 
-    fn change_thread_id(self, thread_id: u32) -> Self {
+    fn change_thread_id(self, thread_id: u64) -> Self {
         PostFixture { thread_id, ..self }
     }
 
-    fn change_post_id(self, post_id: u32) -> Self {
+    fn change_post_id(self, post_id: u64) -> Self {
         PostFixture {
             post_id: Some(post_id),
             ..self
         }
     }
 
-    fn add_post_and_assert(&mut self, result: Result<(), Error>) -> Option<u32> {
+    fn add_post_and_assert(&mut self, result: Result<(), Error>) -> Option<u64> {
         let add_post_result = Discussions::add_post(
             self.origin.clone().into(),
             self.author_id,

runtime/src/lib.rs

@@ -94,6 +94,22 @@ pub type Moment = u64;
 /// Credential type
 pub type Credential = u64;
 
+/// Represents a thread identifier for both Forum and Proposals Discussion
+///
+/// Note: Both modules expose type names ThreadId and PostId (which are defined on their Trait) and
+/// used in state storage and dispatchable method's argument types,
+/// and are therefore part of the public API/metadata of the runtime.
+/// In the currenlty version the polkadot-js/api that is used and is compatible with the runtime,
+/// the type registry has flat namespace and its not possible
+/// to register identically named types from different modules, separately. And so we MUST configure
+/// the underlying types to be identicaly to avoid issues with encoding/decoding these types on the client side.
+pub type ThreadId = u64;
+
+/// Represents a post identifier for both Forum and Proposals Discussion
+///
+/// See the Note about ThreadId
+pub type PostId = u64;
+
 /// Opaque types. These are used by the CLI to instantiate machinery that don't need to know
 /// the specifics of the runtime. They can then be made to be agnostic over specific formats
 /// of data like extrinsics, allowing for them to continue syncing the network through upgrades
@@ -784,6 +800,8 @@ impl forum::ForumUserRegistry<AccountId> for ShimMembershipRegistry {
 impl forum::Trait for Runtime {
     type Event = Event;
     type MembershipRegistry = ShimMembershipRegistry;
+    type ThreadId = ThreadId;
+    type PostId = PostId;
 }
 
 impl migration::Trait for Runtime {
@@ -845,8 +863,8 @@ parameter_types! {
 impl proposals_discussion::Trait for Runtime {
     type Event = Event;
     type PostAuthorOriginValidator = MembershipOriginValidator<Self>;
-    type ThreadId = u32;
-    type PostId = u32;
+    type ThreadId = ThreadId;
+    type PostId = PostId;
     type MaxPostEditionNumber = ProposalMaxPostEditionNumber;
     type ThreadTitleLengthLimit = ProposalThreadTitleLengthLimit;
     type PostLengthLimit = ProposalPostLengthLimit;