Browse Source

runtime-module: blog: multiple minor fixes

conectado 3 years ago
parent
commit
7f698b6f7f

+ 3 - 2
runtime-modules/blog/src/benchmarking.rs

@@ -310,17 +310,18 @@ benchmarks_instance! {
         let post_id = generate_post::<T, I>();
         let (account_id, participant_id) = member_funded_account::<T, I>("caller", 0);
         let mut replies = Vec::new();
+        let hide = false;
 
         for _ in 0..=i {
             let reply_id =
                 generate_reply::<T, I>(account_id.clone(), participant_id, post_id.clone());
-            replies.push((post_id, reply_id, false));
+            replies.push(ReplyToDelete {post_id, reply_id, hide});
         }
 
         let origin = RawOrigin::Signed(account_id);
     }: _(origin.clone(), participant_id, replies.clone())
     verify {
-        for (post_id, reply_id, hide) in replies {
+        for ReplyToDelete {post_id, reply_id, hide} in replies {
             assert!(!<ReplyById<T, I>>::contains_key(post_id, reply_id));
 
             assert_in_events::<T, I>(RawEvent::ReplyDeleted(

+ 18 - 7
runtime-modules/blog/src/lib.rs

@@ -47,7 +47,7 @@ use frame_support::{
 use sp_arithmetic::traits::{BaseArithmetic, One};
 use sp_runtime::SaturatedConversion;
 use sp_runtime::{
-    traits::{AccountIdConversion, Hash, MaybeSerialize, Member},
+    traits::{AccountIdConversion, Hash, MaybeSerialize, Member, Saturating},
     ModuleId,
 };
 use sp_std::prelude::*;
@@ -81,7 +81,7 @@ pub trait WeightInfo {
     fn create_reply_to_post(t: u32) -> Weight;
     fn create_reply_to_reply(t: u32) -> Weight;
     fn edit_reply(t: u32) -> Weight;
-    fn delete_reply() -> Weight;
+    fn delete_replies(i: u32) -> Weight;
 }
 
 type BlogWeightInfo<T, I> = <T as Trait<I>>::WeightInfo;
@@ -332,6 +332,17 @@ impl<T: Trait<I>, I: Instance> Reply<T, I> {
     }
 }
 
+/// Represents a single reply that will be deleted by `delete_replies`
+#[derive(Encode, Decode, Clone, Debug, Default, PartialEq)]
+pub struct ReplyToDelete<ReplyId> {
+    /// Id of the post corresponding to the reply that will be deleted
+    post_id: PostId,
+    /// Id of the reply to be deleted
+    reply_id: ReplyId,
+    /// Whether to hide the reply or just remove it from the storage
+    hide: bool,
+}
+
 // Blog`s pallet storage items.
 decl_storage! {
     trait Store for Module<T: Trait<I>, I: Instance=DefaultInstance> as BlogModule {
@@ -642,20 +653,20 @@ decl_module! {
         /// <weight>
         ///
         /// ## Weight
-        /// `O (1)` doesn't depends on the state or parameters
+        /// `O (1)` doesn't depend on the state or parameters
         /// - DB:
         ///    - O(1) doesn't depend on the state or parameters
         /// # </weight>
-        #[weight = BlogWeightInfo::<T, I>::delete_reply()]
+        #[weight = BlogWeightInfo::<T, I>::delete_replies(replies.len().saturated_into())]
         pub fn delete_replies(
             origin,
             participant_id: ParticipantId<T>,
-            replies: Vec<(PostId, T::ReplyId, bool)>,
+            replies: Vec<ReplyToDelete<T::ReplyId>>,
         ) -> DispatchResult {
             let account_id = Self::ensure_valid_participant(origin, participant_id)?;
 
             let mut erase_replies = Vec::new();
-            for (post_id, reply_id, hide) in replies {
+            for ReplyToDelete { post_id, reply_id, hide } in replies {
                 // Ensure post with given id exists
                 let post = Self::ensure_post_exists(post_id)?;
 
@@ -666,7 +677,7 @@ decl_module! {
                 let reply = Self::ensure_reply_exists(post_id, reply_id)?;
 
                 // Ensure reply -> owner relation exists if post lifetime hasn't ran out
-                if (frame_system::Module::<T>::block_number() - reply.last_edited) <
+                if (frame_system::Module::<T>::block_number().saturating_sub(reply.last_edited)) <
                     T::ReplyLifetime::get()
                 {
                     Self::ensure_reply_ownership(&reply, &participant_id)?;

+ 13 - 9
runtime-modules/blog/src/mock.rs

@@ -12,9 +12,9 @@ use sp_runtime::{
     DispatchResult, Perbill,
 };
 
-pub(crate) const FIRST_OWNER_ORIGIN: u64 = 0;
+pub(crate) const FIRST_OWNER_ORIGIN: u128 = 0;
 pub(crate) const FIRST_OWNER_PARTICIPANT_ID: u64 = 0;
-pub(crate) const SECOND_OWNER_ORIGIN: u64 = 2;
+pub(crate) const SECOND_OWNER_ORIGIN: u128 = 2;
 pub(crate) const SECOND_OWNER_PARTICIPANT_ID: u64 = 2;
 pub(crate) const BAD_MEMBER_ID: u64 = 100000;
 
@@ -55,7 +55,7 @@ impl frame_system::Trait for Runtime {
     type BlockNumber = u64;
     type Hash = H256;
     type Hashing = BlakeTwo256;
-    type AccountId = u64;
+    type AccountId = u128;
     type Lookup = IdentityLookup<Self::AccountId>;
     type Header = Header;
     type Event = TestEvent;
@@ -262,7 +262,7 @@ impl WeightInfo for () {
     fn edit_reply(_: u32) -> Weight {
         unimplemented!()
     }
-    fn delete_reply() -> Weight {
+    fn delete_replies(_: u32) -> Weight {
         unimplemented!()
     }
 }
@@ -406,7 +406,7 @@ pub fn get_reply_text() -> Vec<u8> {
 }
 
 pub fn get_reply(
-    owner: <Runtime as frame_system::Trait>::AccountId,
+    owner: ParticipantId<Runtime>,
     parent_id: ParentId<<Runtime as Trait>::ReplyId, PostId>,
 ) -> Reply<Runtime, DefaultInstance> {
     let reply_text = get_reply_text();
@@ -419,7 +419,7 @@ pub fn get_reply(
 }
 
 pub fn create_reply(
-    origin_id: u64,
+    origin_id: u128,
     participant_id: u64,
     post_id: PostId,
     reply_id: Option<<Runtime as Trait>::ReplyId>,
@@ -437,7 +437,7 @@ pub fn create_reply(
 }
 
 pub fn delete_reply(
-    origin_id: u64,
+    origin_id: u128,
     participant_id: u64,
     post_id: PostId,
     reply_id: <Runtime as Trait>::ReplyId,
@@ -445,12 +445,16 @@ pub fn delete_reply(
     TestBlogModule::delete_replies(
         Origin::signed(origin_id),
         participant_id,
-        vec![(post_id, reply_id, false)],
+        vec![ReplyToDelete {
+            post_id,
+            reply_id,
+            hide: false,
+        }],
     )
 }
 
 pub fn edit_reply(
-    origin_id: u64,
+    origin_id: u128,
     participant_id: u64,
     post_id: PostId,
     reply_id: <Runtime as Trait>::ReplyId,

+ 11 - 30
runtime-modules/blog/src/tests.rs

@@ -3,7 +3,6 @@
 use crate::mock::*;
 use crate::*;
 use frame_support::assert_ok;
-use frame_system::ensure_signed;
 
 //Blog, post or reply id
 const FIRST_ID: u64 = 0;
@@ -39,7 +38,7 @@ fn assert_failure(
 
 fn ensure_replies_equality(
     reply: Option<Reply<Runtime, DefaultInstance>>,
-    reply_owner_id: <Runtime as frame_system::Trait>::AccountId,
+    reply_owner_id: ParticipantId<Runtime>,
     parent: ParentId<<Runtime as Trait>::ReplyId, PostId>,
 ) {
     // Ensure  stored reply is equal to expected one
@@ -420,8 +419,6 @@ fn editable_reply_creation_success() {
             <Runtime as Trait>::ReplyDeposit::get()
         );
 
-        let reply_owner_id = ensure_signed(Origin::signed(SECOND_OWNER_ORIGIN)).unwrap();
-
         // Events number before tested call
         let number_of_events_before_call = System::events().len();
 
@@ -442,14 +439,14 @@ fn editable_reply_creation_success() {
         // Replies related storage updated succesfully
         let reply = reply_by_id(FIRST_ID, FIRST_ID);
 
-        ensure_replies_equality(reply, reply_owner_id, ParentId::Post(FIRST_ID));
+        ensure_replies_equality(reply, SECOND_OWNER_PARTICIPANT_ID, ParentId::Post(FIRST_ID));
 
         // Root replies counter updated
         assert_eq!(post.replies_count(), 1);
 
         // Event checked
         let reply_created_event = get_test_event(RawEvent::ReplyCreated(
-            reply_owner_id,
+            SECOND_OWNER_PARTICIPANT_ID,
             FIRST_ID,
             FIRST_ID,
             get_reply_text(),
@@ -489,8 +486,6 @@ fn non_editable_reply_creation_success() {
         // Create post for future replies
         create_post(Origin::root()).unwrap();
 
-        let reply_owner_id = ensure_signed(Origin::signed(SECOND_OWNER_ORIGIN)).unwrap();
-
         // Events number before tested call
         let number_of_events_before_call = System::events().len();
 
@@ -513,7 +508,7 @@ fn non_editable_reply_creation_success() {
 
         // Event checked
         let reply_created_event = get_test_event(RawEvent::ReplyCreated(
-            reply_owner_id,
+            SECOND_OWNER_PARTICIPANT_ID,
             FIRST_ID,
             FIRST_ID,
             get_reply_text(),
@@ -539,8 +534,6 @@ fn editable_direct_reply_creation_success() {
             <Runtime as Trait>::ReplyDeposit::get(),
         );
 
-        let direct_reply_owner_id = ensure_signed(Origin::signed(SECOND_OWNER_ORIGIN)).unwrap();
-
         assert_ok!(create_reply(
             FIRST_OWNER_ORIGIN,
             FIRST_OWNER_PARTICIPANT_ID,
@@ -573,7 +566,7 @@ fn editable_direct_reply_creation_success() {
 
         // Event checked
         let reply_created_event = get_test_event(RawEvent::DirectReplyCreated(
-            direct_reply_owner_id,
+            SECOND_OWNER_PARTICIPANT_ID,
             FIRST_ID,
             FIRST_ID,
             SECOND_ID,
@@ -596,8 +589,6 @@ fn non_editable_direct_reply_creation_success() {
             <Runtime as Trait>::ReplyDeposit::get(),
         );
 
-        let direct_reply_owner_id = ensure_signed(Origin::signed(SECOND_OWNER_ORIGIN)).unwrap();
-
         assert_ok!(create_reply(
             FIRST_OWNER_ORIGIN,
             FIRST_OWNER_PARTICIPANT_ID,
@@ -629,7 +620,7 @@ fn non_editable_direct_reply_creation_success() {
 
         // Event checked
         let reply_created_event = get_test_event(RawEvent::DirectReplyCreated(
-            direct_reply_owner_id,
+            SECOND_OWNER_PARTICIPANT_ID,
             FIRST_ID,
             FIRST_ID,
             SECOND_ID,
@@ -652,8 +643,6 @@ fn editable_direct_reply_to_non_editable_reply_creation_success() {
             <Runtime as Trait>::ReplyDeposit::get(),
         );
 
-        let direct_reply_owner_id = ensure_signed(Origin::signed(SECOND_OWNER_ORIGIN)).unwrap();
-
         assert_ok!(create_reply(
             FIRST_OWNER_ORIGIN,
             FIRST_OWNER_PARTICIPANT_ID,
@@ -686,7 +675,7 @@ fn editable_direct_reply_to_non_editable_reply_creation_success() {
 
         // Event checked
         let reply_created_event = get_test_event(RawEvent::DirectReplyCreated(
-            direct_reply_owner_id,
+            SECOND_OWNER_PARTICIPANT_ID,
             FIRST_ID,
             FIRST_ID,
             SECOND_ID,
@@ -796,8 +785,6 @@ fn reply_editing_success() {
         // Create post for future replies
         create_post(Origin::root()).unwrap();
 
-        let reply_owner_id = ensure_signed(Origin::signed(SECOND_OWNER_ORIGIN)).unwrap();
-
         Balances::<Runtime>::make_free_balance_be(
             &SECOND_OWNER_ORIGIN,
             <Runtime as Trait>::ReplyDeposit::get(),
@@ -826,7 +813,7 @@ fn reply_editing_success() {
         // Reply after editing checked
         let reply = reply_by_id(FIRST_ID, FIRST_ID);
 
-        ensure_replies_equality(reply, reply_owner_id, ParentId::Post(FIRST_ID));
+        ensure_replies_equality(reply, SECOND_OWNER_PARTICIPANT_ID, ParentId::Post(FIRST_ID));
 
         // Event checked
         let reply_edited_event = get_test_event(RawEvent::ReplyEdited(
@@ -845,8 +832,6 @@ fn reply_editing_post_locked_error() {
         // Create post for future replies
         create_post(Origin::root()).unwrap();
 
-        let reply_owner_id = ensure_signed(Origin::signed(SECOND_OWNER_ORIGIN)).unwrap();
-
         Balances::<Runtime>::make_free_balance_be(
             &SECOND_OWNER_ORIGIN,
             <Runtime as Trait>::ReplyDeposit::get(),
@@ -878,7 +863,7 @@ fn reply_editing_post_locked_error() {
         let reply = reply_by_id(FIRST_ID, FIRST_ID);
 
         // Compare with default unedited reply
-        ensure_replies_equality(reply, reply_owner_id, ParentId::Post(FIRST_ID));
+        ensure_replies_equality(reply, SECOND_OWNER_PARTICIPANT_ID, ParentId::Post(FIRST_ID));
 
         // Failure checked
         assert_failure(
@@ -925,8 +910,6 @@ fn reply_editing_ownership_error() {
             <Runtime as Trait>::ReplyDeposit::get(),
         );
 
-        let reply_owner_id = ensure_signed(Origin::signed(SECOND_OWNER_ORIGIN)).unwrap();
-
         create_reply(
             SECOND_OWNER_ORIGIN,
             SECOND_OWNER_PARTICIPANT_ID,
@@ -950,7 +933,7 @@ fn reply_editing_ownership_error() {
         let reply = reply_by_id(FIRST_ID, FIRST_ID);
 
         // Compare with default unedited reply
-        ensure_replies_equality(reply, reply_owner_id, ParentId::Post(FIRST_ID));
+        ensure_replies_equality(reply, SECOND_OWNER_PARTICIPANT_ID, ParentId::Post(FIRST_ID));
 
         // Failure checked
         assert_failure(
@@ -986,8 +969,6 @@ fn reply_editing_participant_error() {
         // Create post for future replies
         create_post(Origin::root()).unwrap();
 
-        let reply_owner_id = ensure_signed(Origin::signed(SECOND_OWNER_ORIGIN)).unwrap();
-
         Balances::<Runtime>::make_free_balance_be(
             &SECOND_OWNER_ORIGIN,
             <Runtime as Trait>::ReplyDeposit::get(),
@@ -1012,7 +993,7 @@ fn reply_editing_participant_error() {
         let reply = reply_by_id(FIRST_ID, FIRST_ID);
 
         // Compare with default unedited reply
-        ensure_replies_equality(reply, reply_owner_id, ParentId::Post(FIRST_ID));
+        ensure_replies_equality(reply, SECOND_OWNER_PARTICIPANT_ID, ParentId::Post(FIRST_ID));
 
         // Failure checked
         assert_failure(

+ 17 - 16
runtime/src/weights/blog.rs

@@ -8,49 +8,50 @@ use frame_support::weights::{constants::RocksDbWeight as DbWeight, Weight};
 pub struct WeightInfo;
 impl blog::WeightInfo for WeightInfo {
     fn create_post(t: u32, b: u32) -> Weight {
-        (291_037_000 as Weight)
-            .saturating_add((101_000 as Weight).saturating_mul(t as Weight))
-            .saturating_add((139_000 as Weight).saturating_mul(b as Weight))
+        (317_873_000 as Weight)
+            .saturating_add((92_000 as Weight).saturating_mul(t as Weight))
+            .saturating_add((129_000 as Weight).saturating_mul(b as Weight))
             .saturating_add(DbWeight::get().reads(1 as Weight))
             .saturating_add(DbWeight::get().writes(2 as Weight))
     }
     fn lock_post() -> Weight {
-        (166_327_000 as Weight)
+        (160_756_000 as Weight)
             .saturating_add(DbWeight::get().reads(1 as Weight))
             .saturating_add(DbWeight::get().writes(1 as Weight))
     }
     fn unlock_post() -> Weight {
-        (164_654_000 as Weight)
+        (158_932_000 as Weight)
             .saturating_add(DbWeight::get().reads(1 as Weight))
             .saturating_add(DbWeight::get().writes(1 as Weight))
     }
     fn edit_post(t: u32, b: u32) -> Weight {
-        (464_651_000 as Weight)
-            .saturating_add((99_000 as Weight).saturating_mul(t as Weight))
-            .saturating_add((134_000 as Weight).saturating_mul(b as Weight))
+        (384_273_000 as Weight)
+            .saturating_add((95_000 as Weight).saturating_mul(t as Weight))
+            .saturating_add((131_000 as Weight).saturating_mul(b as Weight))
             .saturating_add(DbWeight::get().reads(1 as Weight))
             .saturating_add(DbWeight::get().writes(1 as Weight))
     }
     fn create_reply_to_post(t: u32) -> Weight {
-        (594_751_000 as Weight)
-            .saturating_add((174_000 as Weight).saturating_mul(t as Weight))
+        (591_784_000 as Weight)
+            .saturating_add((161_000 as Weight).saturating_mul(t as Weight))
             .saturating_add(DbWeight::get().reads(3 as Weight))
             .saturating_add(DbWeight::get().writes(3 as Weight))
     }
     fn create_reply_to_reply(t: u32) -> Weight {
-        (561_603_000 as Weight)
-            .saturating_add((169_000 as Weight).saturating_mul(t as Weight))
+        (508_330_000 as Weight)
+            .saturating_add((160_000 as Weight).saturating_mul(t as Weight))
             .saturating_add(DbWeight::get().reads(3 as Weight))
             .saturating_add(DbWeight::get().writes(3 as Weight))
     }
     fn edit_reply(t: u32) -> Weight {
-        (302_109_000 as Weight)
-            .saturating_add((170_000 as Weight).saturating_mul(t as Weight))
+        (281_077_000 as Weight)
+            .saturating_add((160_000 as Weight).saturating_mul(t as Weight))
             .saturating_add(DbWeight::get().reads(3 as Weight))
             .saturating_add(DbWeight::get().writes(1 as Weight))
     }
-    fn delete_reply() -> Weight {
-        (501_946_000 as Weight)
+    fn delete_replies(i: u32) -> Weight {
+        (492_001_000 as Weight)
+            .saturating_add((352_154_000 as Weight).saturating_mul(i as Weight))
             .saturating_add(DbWeight::get().reads(4 as Weight))
             .saturating_add(DbWeight::get().writes(2 as Weight))
     }