Browse Source

runtime: bounty: Add ‘cancel_bounty’ extrinsic.

Shamil Gadelshin 4 years ago
parent
commit
41f73a0581

+ 32 - 2
runtime-modules/bounty/src/benchmarking.rs

@@ -1,13 +1,15 @@
 #![cfg(feature = "runtime-benchmarks")]
 
 use frame_benchmarking::benchmarks;
+use frame_support::storage::StorageMap;
 use frame_system::Module as System;
 use frame_system::{EventRecord, RawOrigin};
+use sp_arithmetic::traits::One;
 use sp_std::boxed::Box;
 use sp_std::vec;
 use sp_std::vec::Vec;
 
-use crate::{Bounty, BountyCreationParameters, Call, Event, Module, Trait};
+use crate::{Bounties, Bounty, BountyCreationParameters, Call, Event, Module, Trait};
 
 fn assert_last_event<T: Trait>(generic_event: <T as Trait>::Event) {
     let events = System::<T>::events();
@@ -26,7 +28,11 @@ benchmarks! {
         let i in 1 .. MAX_BYTES;
         let metadata = vec![0u8].repeat(i as usize);
 
-        let params = BountyCreationParameters::<T>::default();
+        let params = BountyCreationParameters::<T>{
+            work_period: One::one(),
+            judging_period: One::one(),
+            ..Default::default()
+        };
 
     }: _ (RawOrigin::Root, params.clone(), metadata)
     verify {
@@ -39,6 +45,23 @@ benchmarks! {
         assert_eq!(Module::<T>::bounties(bounty_id), bounty);
         assert_last_event::<T>(Event::<T>::BountyCreated(bounty_id).into());
     }
+
+    cancel_bounty{
+        let params = BountyCreationParameters::<T>{
+            work_period: One::one(),
+            judging_period: One::one(),
+            ..Default::default()
+        };
+
+        Module::<T>::create_bounty(RawOrigin::Root.into(), params, Vec::new()).unwrap();
+
+        let bounty_id: T::BountyId = Module::<T>::bounty_count().into();
+
+    }: _ (RawOrigin::Root, None, bounty_id)
+    verify {
+        assert!(!<Bounties<T>>::contains_key(&bounty_id));
+        assert_last_event::<T>(Event::<T>::BountyCanceled(bounty_id).into());
+    }
 }
 
 #[cfg(test)]
@@ -53,4 +76,11 @@ mod tests {
             assert_ok!(test_benchmark_create_bounty::<Test>());
         });
     }
+
+    #[test]
+    fn cancel_bounty() {
+        build_test_externalities().execute_with(|| {
+            assert_ok!(test_benchmark_cancel_bounty::<Test>());
+        });
+    }
 }

+ 93 - 6
runtime-modules/bounty/src/lib.rs

@@ -4,7 +4,8 @@
 //! A detailed description could be found [here](https://github.com/Joystream/joystream/issues/1998).
 //!
 //! ### Supported extrinsics:
-//! - [create_bounty](./struct.Module.html#create_bounty.vote) - creates a bounty
+//! - [create_bounty](./struct.Module.html#method.create_bounty) - creates a bounty
+//! - [cancel_bounty](./struct.Module.html#method.cancel_bounty) - cancels a bounty
 
 // Ensure we're `no_std` when compiling for Wasm.
 #![cfg_attr(not(feature = "std"), no_std)]
@@ -21,22 +22,23 @@ mod benchmarking;
 /// pallet_bounty WeightInfo.
 /// Note: This was auto generated through the benchmark CLI using the `--weight-trait` flag
 pub trait WeightInfo {
-    fn create_bounty(i: u32) -> Weight;
+    fn create_bounty() -> Weight;
+    fn cancel_bounty() -> Weight;
 }
 
 type WeightInfoBounty<T> = <T as Trait>::WeightInfo;
 
+use frame_support::dispatch::DispatchResult;
 use frame_support::weights::Weight;
 use frame_support::{decl_error, decl_event, decl_module, decl_storage, ensure, Parameter};
 use frame_system::ensure_root;
-use sp_runtime::SaturatedConversion;
+use sp_arithmetic::traits::Zero;
 use sp_std::vec::Vec;
 
 use common::origin::MemberOriginValidator;
 use common::MemberId;
 
 use codec::{Decode, Encode};
-use frame_support::dispatch::DispatchResult;
 #[cfg(feature = "std")]
 use serde::{Deserialize, Serialize};
 
@@ -171,6 +173,9 @@ decl_event! {
     {
         /// A bounty was created.
         BountyCreated(BountyId),
+
+        /// A bounty was canceled.
+        BountyCanceled(BountyId),
     }
 }
 
@@ -179,22 +184,41 @@ decl_error! {
     pub enum Error for Module<T: Trait> {
         /// Min funding amount cannot be greater than max amount.
         MinFundingAmountCannotBeGreaterThanMaxAmount,
+
+        /// Bounty doesnt exist.
+        BountyDoesntExist,
+
+        /// Operation can be performed only by a bounty creator.
+        NotBountyCreator,
+
+        /// Work period cannot be zero.
+        WorkPeriodCannotBeZero,
+
+        /// Judging period cannot be zero.
+        JudgingPeriodCannotBeZero,
     }
 }
 
 decl_module! {
     pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+        /// Predefined errors
+        type Error = Error<T>;
+
+        /// Emits an event. Default substrate implementation.
         fn deposit_event() = default;
 
         /// Creates a bounty. Metadata stored in the transaction log but discarded after that.
-        #[weight = WeightInfoBounty::<T>::create_bounty(_metadata.len().saturated_into())]
-        fn create_bounty(origin, params: BountyCreationParameters<T>, _metadata: Vec<u8>) {
+        #[weight = WeightInfoBounty::<T>::create_bounty()]
+        pub fn create_bounty(origin, params: BountyCreationParameters<T>, _metadata: Vec<u8>) {
             Self::ensure_create_bounty_parameters_valid(&origin, &params)?;
 
             //
             // == MUTATION SAFE ==
             //
 
+            // TODO: add creation block
+            // TODO: slash cherry from the balance
+
             let next_bounty_count_value = Self::bounty_count() + 1;
             let bounty_id = T::BountyId::from(next_bounty_count_value);
 
@@ -208,6 +232,21 @@ decl_module! {
             });
             Self::deposit_event(RawEvent::BountyCreated(bounty_id));
         }
+
+        /// Cancels a bounty.
+        #[weight = WeightInfoBounty::<T>::cancel_bounty()]
+        pub fn cancel_bounty(origin, creator_member_id: Option<MemberId<T>>, bounty_id: T::BountyId) {
+            Self::ensure_cancel_bounty_parameters_valid(&origin, creator_member_id, bounty_id)?;
+
+            //
+            // == MUTATION SAFE ==
+            //
+
+            // TODO: make payments for submitted work.
+
+            <Bounties<T>>::remove(bounty_id);
+            Self::deposit_event(RawEvent::BountyCanceled(bounty_id));
+        }
     }
 }
 
@@ -227,6 +266,16 @@ impl<T: Trait> Module<T> {
             ensure_root(origin.clone())?;
         }
 
+        ensure!(
+            params.work_period != Zero::zero(),
+            Error::<T>::WorkPeriodCannotBeZero
+        );
+
+        ensure!(
+            params.judging_period != Zero::zero(),
+            Error::<T>::JudgingPeriodCannotBeZero
+        );
+
         ensure!(
             params.min_amount <= params.max_amount,
             Error::<T>::MinFundingAmountCannotBeGreaterThanMaxAmount
@@ -234,4 +283,42 @@ impl<T: Trait> Module<T> {
 
         Ok(())
     }
+
+    // Validates parameters for a bounty cancellation.
+    fn ensure_cancel_bounty_parameters_valid(
+        origin: &T::Origin,
+        creator_member_id: Option<MemberId<T>>,
+        bounty_id: T::BountyId,
+    ) -> DispatchResult {
+        ensure!(
+            <Bounties<T>>::contains_key(bounty_id),
+            Error::<T>::BountyDoesntExist
+        );
+
+        let bounty = <Bounties<T>>::get(bounty_id);
+
+        // Validate origin.
+        if let Some(member_id) = creator_member_id {
+            T::MemberOriginValidator::ensure_member_controller_account_origin(
+                origin.clone(),
+                member_id,
+            )?;
+
+            ensure!(
+                bounty.creation_params.creator_member_id == creator_member_id,
+                Error::<T>::NotBountyCreator,
+            );
+        } else {
+            ensure_root(origin.clone())?;
+
+            ensure!(
+                bounty.creation_params.creator_member_id.is_none(),
+                Error::<T>::NotBountyCreator,
+            );
+        }
+
+        // TODO: check bounty stage
+
+        Ok(())
+    }
 }

+ 66 - 0
runtime-modules/bounty/src/tests/fixtures.rs

@@ -42,6 +42,8 @@ pub struct CreateBountyFixture {
     metadata: Vec<u8>,
     creator_member_id: Option<u64>,
     min_amount: u64,
+    work_period: u64,
+    judging_period: u64,
 }
 
 impl CreateBountyFixture {
@@ -51,6 +53,8 @@ impl CreateBountyFixture {
             metadata: Vec::new(),
             creator_member_id: None,
             min_amount: 0,
+            work_period: 1,
+            judging_period: 1,
         }
     }
 
@@ -68,14 +72,31 @@ impl CreateBountyFixture {
     pub fn with_metadata(self, metadata: Vec<u8>) -> Self {
         Self { metadata, ..self }
     }
+
     pub fn with_min_amount(self, min_amount: u64) -> Self {
         Self { min_amount, ..self }
     }
 
+    pub fn with_work_period(self, work_period: u64) -> Self {
+        Self {
+            work_period,
+            ..self
+        }
+    }
+
+    pub fn with_judging_period(self, judging_period: u64) -> Self {
+        Self {
+            judging_period,
+            ..self
+        }
+    }
+
     pub fn call_and_assert(&self, expected_result: DispatchResult) {
         let params = BountyCreationParameters::<Test> {
             creator_member_id: self.creator_member_id.clone(),
             min_amount: self.min_amount.clone(),
+            work_period: self.work_period.clone(),
+            judging_period: self.judging_period.clone(),
             ..Default::default()
         };
 
@@ -96,3 +117,48 @@ impl CreateBountyFixture {
         }
     }
 }
+
+pub struct CancelBountyFixture {
+    origin: RawOrigin<u64>,
+    creator_member_id: Option<u64>,
+    bounty_id: u64,
+}
+
+impl CancelBountyFixture {
+    pub fn default() -> Self {
+        Self {
+            origin: RawOrigin::Root,
+            creator_member_id: None,
+            bounty_id: 1,
+        }
+    }
+
+    pub fn with_origin(self, origin: RawOrigin<u64>) -> Self {
+        Self { origin, ..self }
+    }
+
+    pub fn with_creator_member_id(self, member_id: u64) -> Self {
+        Self {
+            creator_member_id: Some(member_id),
+            ..self
+        }
+    }
+
+    pub fn with_bounty_id(self, bounty_id: u64) -> Self {
+        Self { bounty_id, ..self }
+    }
+
+    pub fn call_and_assert(&self, expected_result: DispatchResult) {
+        let actual_result = Bounty::cancel_bounty(
+            self.origin.clone().into(),
+            self.creator_member_id.clone(),
+            self.bounty_id.clone(),
+        );
+
+        assert_eq!(actual_result, expected_result);
+
+        if actual_result.is_ok() {
+            assert!(!<crate::Bounties<Test>>::contains_key(&self.bounty_id));
+        }
+    }
+}

+ 4 - 1
runtime-modules/bounty/src/tests/mocks.rs

@@ -73,7 +73,10 @@ impl Trait for Test {
 }
 
 impl crate::WeightInfo for () {
-    fn create_bounty(_: u32) -> u64 {
+    fn create_bounty() -> u64 {
+        0
+    }
+    fn cancel_bounty() -> u64 {
         0
     }
 }

+ 117 - 13
runtime-modules/bounty/src/tests/mod.rs

@@ -7,7 +7,7 @@ use frame_system::RawOrigin;
 use sp_runtime::DispatchError;
 
 use crate::{Error, RawEvent};
-use fixtures::{run_to_block, CreateBountyFixture, EventFixture};
+use fixtures::{run_to_block, CancelBountyFixture, CreateBountyFixture, EventFixture};
 use mocks::{build_test_externalities, Test};
 
 #[test]
@@ -18,8 +18,9 @@ fn create_bounty_succeeds() {
 
         let text = b"Bounty text".to_vec();
 
-        let create_bounty_fixture = CreateBountyFixture::default().with_metadata(text);
-        create_bounty_fixture.call_and_assert(Ok(()));
+        CreateBountyFixture::default()
+            .with_metadata(text)
+            .call_and_assert(Ok(()));
 
         let bounty_id = 1u64;
 
@@ -31,24 +32,127 @@ fn create_bounty_succeeds() {
 fn create_bounty_fails_with_invalid_origin() {
     build_test_externalities().execute_with(|| {
         // For a council bounty.
-        let create_bounty_fixture =
-            CreateBountyFixture::default().with_origin(RawOrigin::Signed(1));
-        create_bounty_fixture.call_and_assert(Err(DispatchError::BadOrigin));
+        CreateBountyFixture::default()
+            .with_origin(RawOrigin::Signed(1))
+            .call_and_assert(Err(DispatchError::BadOrigin));
 
         // For a member bounty.
-        let create_bounty_fixture = CreateBountyFixture::default()
+        CreateBountyFixture::default()
             .with_origin(RawOrigin::Root)
-            .with_creator_member_id(1);
-        create_bounty_fixture.call_and_assert(Err(DispatchError::BadOrigin));
+            .with_creator_member_id(1)
+            .call_and_assert(Err(DispatchError::BadOrigin));
     });
 }
 
 #[test]
 fn create_bounty_fails_with_invalid_min_max_amounts() {
     build_test_externalities().execute_with(|| {
-        let create_bounty_fixture = CreateBountyFixture::default().with_min_amount(100);
-        create_bounty_fixture.call_and_assert(Err(
-            Error::<Test>::MinFundingAmountCannotBeGreaterThanMaxAmount.into(),
-        ));
+        CreateBountyFixture::default()
+            .with_min_amount(100)
+            .call_and_assert(Err(
+                Error::<Test>::MinFundingAmountCannotBeGreaterThanMaxAmount.into(),
+            ));
+    });
+}
+
+#[test]
+fn create_bounty_fails_with_invalid_periods() {
+    build_test_externalities().execute_with(|| {
+        CreateBountyFixture::default()
+            .with_work_period(0)
+            .call_and_assert(Err(Error::<Test>::WorkPeriodCannotBeZero.into()));
+
+        CreateBountyFixture::default()
+            .with_judging_period(0)
+            .call_and_assert(Err(Error::<Test>::JudgingPeriodCannotBeZero.into()));
+    });
+}
+
+#[test]
+fn cancel_bounty_succeeds() {
+    build_test_externalities().execute_with(|| {
+        let starting_block = 1;
+        run_to_block(starting_block);
+
+        CreateBountyFixture::default().call_and_assert(Ok(()));
+
+        let bounty_id = 1u64;
+
+        CancelBountyFixture::default()
+            .with_bounty_id(bounty_id)
+            .call_and_assert(Ok(()));
+
+        EventFixture::assert_last_crate_event(RawEvent::BountyCanceled(bounty_id));
+    });
+}
+
+#[test]
+fn cancel_bounty_fails_with_invalid_bounty_id() {
+    build_test_externalities().execute_with(|| {
+        let invalid_bounty_id = 11u64;
+
+        CancelBountyFixture::default()
+            .with_bounty_id(invalid_bounty_id)
+            .call_and_assert(Err(Error::<Test>::BountyDoesntExist.into()));
+    });
+}
+
+#[test]
+fn cancel_bounty_fails_with_invalid_origin() {
+    build_test_externalities().execute_with(|| {
+        let member_id = 1;
+        let account_id = 1;
+
+        // Created by council - try to cancel with bad origin
+        CreateBountyFixture::default()
+            .with_origin(RawOrigin::Root)
+            .call_and_assert(Ok(()));
+
+        let bounty_id = 1u64;
+
+        CancelBountyFixture::default()
+            .with_bounty_id(bounty_id)
+            .with_origin(RawOrigin::Signed(account_id))
+            .call_and_assert(Err(DispatchError::BadOrigin));
+
+        // Created by a member - try to cancel with invalid member_id
+        CreateBountyFixture::default()
+            .with_origin(RawOrigin::Signed(account_id))
+            .with_creator_member_id(member_id)
+            .call_and_assert(Ok(()));
+
+        let bounty_id = 2u64;
+        let invalid_member_id = 2;
+
+        CancelBountyFixture::default()
+            .with_bounty_id(bounty_id)
+            .with_origin(RawOrigin::Signed(account_id))
+            .with_creator_member_id(invalid_member_id)
+            .call_and_assert(Err(Error::<Test>::NotBountyCreator.into()));
+
+        // Created by a member - try to cancel with bad origin
+        CreateBountyFixture::default()
+            .with_origin(RawOrigin::Signed(1))
+            .with_creator_member_id(member_id)
+            .call_and_assert(Ok(()));
+
+        let bounty_id = 3u64;
+
+        CancelBountyFixture::default()
+            .with_bounty_id(bounty_id)
+            .with_origin(RawOrigin::None)
+            .call_and_assert(Err(DispatchError::BadOrigin));
+
+        // Created by a member  - try to cancel by council
+        CreateBountyFixture::default()
+            .with_origin(RawOrigin::Signed(account_id))
+            .with_creator_member_id(member_id)
+            .call_and_assert(Ok(()));
+
+        let bounty_id = 4u64;
+        CancelBountyFixture::default()
+            .with_bounty_id(bounty_id)
+            .with_origin(RawOrigin::Root)
+            .call_and_assert(Err(Error::<Test>::NotBountyCreator.into()));
     });
 }

+ 1 - 1
runtime-modules/constitution/src/lib.rs

@@ -71,7 +71,7 @@ decl_module! {
         /// - Db writes: 1 (constant value)
         /// # </weight>
         #[weight = WeightInfoConstitution::<T>::amend_constitution(constitution_text.len().saturated_into())]
-        fn amend_constitution(origin, constitution_text: Vec<u8>) {
+        pub fn amend_constitution(origin, constitution_text: Vec<u8>) {
             ensure_root(origin)?;
 
             //

+ 8 - 3
runtime/src/weights/bounty.rs

@@ -7,10 +7,15 @@ use frame_support::weights::{constants::RocksDbWeight as DbWeight, Weight};
 
 pub struct WeightInfo;
 impl bounty::WeightInfo for WeightInfo {
-    fn create_bounty(i: u32) -> Weight {
-        (71_128_000 as Weight)
-            .saturating_add((125_000 as Weight).saturating_mul(i as Weight))
+    // WARNING! Some components were not used: ["i"]
+    fn create_bounty() -> Weight {
+        (147_346_000 as Weight)
             .saturating_add(DbWeight::get().reads(1 as Weight))
             .saturating_add(DbWeight::get().writes(2 as Weight))
     }
+    fn cancel_bounty() -> Weight {
+        (191_000_000 as Weight)
+            .saturating_add(DbWeight::get().reads(1 as Weight))
+            .saturating_add(DbWeight::get().writes(1 as Weight))
+    }
 }