Browse Source

Merge branch 'development' into content-wg-replace-lead

Mokhtar Naamani 5 years ago
parent
commit
e150d54337

+ 4 - 0
Cargo.lock

@@ -1554,7 +1554,11 @@ dependencies = [
 
 [[package]]
 name = "joystream-node"
+<<<<<<< HEAD
 version = "2.1.5"
+=======
+version = "2.1.4"
+>>>>>>> development
 dependencies = [
  "ctrlc",
  "derive_more 0.14.1",

+ 3 - 3
runtime-modules/content-working-group/src/genesis.rs

@@ -43,11 +43,11 @@ pub struct GenesisConfigBuilder<T: Trait> {
 }
 
 impl<T: Trait> GenesisConfigBuilder<T> {
-    /*
-    pub fn set_mint(mut self, mint: <T as minting::Trait>::MintId) -> Self {
-        self.mint = mint;
+    pub fn with_mint_capacity(mut self, capacity: minting::BalanceOf<T>) -> Self {
+        self.mint_capacity = capacity;
         self
     }
+    /*
     pub fn set_channel_handle_constraint(mut self, constraint: InputValidationLengthConstraint) -> Self {
         self.channel_description_constraint = constraint;
         self

+ 45 - 2
runtime-modules/content-working-group/src/lib.rs

@@ -1080,7 +1080,9 @@ decl_event! {
         CuratorApplicationId = CuratorApplicationId<T>,
         CuratorId = CuratorId<T>,
         CuratorApplicationIdToCuratorIdMap = CuratorApplicationIdToCuratorIdMap<T>,
+        MintBalanceOf = minting::BalanceOf<T>,
         <T as system::Trait>::AccountId,
+        <T as minting::Trait>::MintId,
     {
         ChannelCreated(ChannelId),
         ChannelOwnershipTransferred(ChannelId),
@@ -1100,6 +1102,8 @@ decl_event! {
         CuratorRewardAccountUpdated(CuratorId, AccountId),
         ChannelUpdatedByCurationActor(ChannelId),
         ChannelCreationEnabledUpdated(bool),
+        MintCapacityIncreased(MintId, MintBalanceOf, MintBalanceOf),
+        MintCapacityDecreased(MintId, MintBalanceOf, MintBalanceOf),
     }
 }
 
@@ -1942,7 +1946,11 @@ decl_module! {
             Self::deposit_event(RawEvent::ChannelCreationEnabledUpdated(enabled));
         }
 
-        /// Add to capacity of current acive mint
+        /// Add to capacity of current acive mint.
+        /// This may be deprecated in the future, since set_mint_capacity is sufficient to
+        /// both increase and decrease capacity. Although when considering that it may be executed
+        /// by a proposal, given the temporal delay in approving a proposal, it might be more suitable
+        /// than set_mint_capacity?
         pub fn increase_mint_capacity(
             origin,
             additional_capacity: minting::BalanceOf<T>
@@ -1952,7 +1960,42 @@ decl_module! {
             let mint_id = Self::mint();
             let mint = <minting::Module<T>>::mints(mint_id); // must exist
             let new_capacity = mint.capacity() + additional_capacity;
-            let _ = <minting::Module<T>>::set_mint_capacity(mint_id, new_capacity);
+            <minting::Module<T>>::set_mint_capacity(mint_id, new_capacity)?;
+
+            Self::deposit_event(RawEvent::MintCapacityIncreased(
+                mint_id, additional_capacity, new_capacity
+            ));
+        }
+
+        /// Sets the capacity of the current active mint
+        pub fn set_mint_capacity(
+            origin,
+            new_capacity: minting::BalanceOf<T>
+        ) {
+            ensure_root(origin)?;
+
+            let mint_id = Self::mint();
+
+            // Mint must exist - it is set at genesis
+            let mint = <minting::Module<T>>::mints(mint_id);
+
+            let current_capacity = mint.capacity();
+
+            if new_capacity != current_capacity {
+                // Cannot fail if mint exists
+                <minting::Module<T>>::set_mint_capacity(mint_id, new_capacity)?;
+
+                if new_capacity > current_capacity {
+                    Self::deposit_event(RawEvent::MintCapacityIncreased(
+                        mint_id, new_capacity - current_capacity, new_capacity
+                    ));
+                } else {
+                    Self::deposit_event(RawEvent::MintCapacityDecreased(
+                        mint_id, current_capacity - new_capacity, new_capacity
+                    ));
+                }
+            }
+
         }
     }
 }

+ 7 - 2
runtime-modules/content-working-group/src/mock.rs

@@ -68,7 +68,9 @@ pub type RawLibTestEvent = RawEvent<
     CuratorApplicationId<Test>,
     CuratorId<Test>,
     CuratorApplicationIdToCuratorIdMap<Test>,
+    minting::BalanceOf<Test>,
     <Test as system::Trait>::AccountId,
+    <Test as minting::Trait>::MintId,
 >;
 
 pub fn get_last_event_or_panic() -> RawLibTestEvent {
@@ -220,11 +222,13 @@ impl<T: Trait> TestExternalitiesBuilder<T> {
         self.membership_config = Some(membership_config);
         self
     }
-    pub fn set_content_wg_config(mut self, conteng_wg_config: GenesisConfig<T>) -> Self {
+    */
+
+    pub fn with_content_wg_config(mut self, conteng_wg_config: GenesisConfig<T>) -> Self {
         self.content_wg_config = Some(conteng_wg_config);
         self
     }
-    */
+
     pub fn build(self) -> runtime_io::TestExternalities {
         // Add system
         let mut t = self
@@ -260,3 +264,4 @@ impl<T: Trait> TestExternalitiesBuilder<T> {
 pub type System = system::Module<Test>;
 pub type Balances = balances::Module<Test>;
 pub type ContentWorkingGroup = Module<Test>;
+pub type Minting = minting::Module<Test>;

+ 85 - 1
runtime-modules/content-working-group/src/tests.rs

@@ -1,6 +1,6 @@
 #![cfg(test)]
 
-//use super::genesis;
+use super::genesis;
 use super::mock::{self, *};
 //use crate::membership;
 use hiring;
@@ -2186,3 +2186,87 @@ pub fn generate_too_short_length_buffer(constraint: &InputValidationLengthConstr
 pub fn generate_too_long_length_buffer(constraint: &InputValidationLengthConstraint) -> Vec<u8> {
     generate_text((constraint.max() + 1) as usize)
 }
+
+#[test]
+fn increasing_mint_capacity() {
+    const MINT_CAPACITY: u64 = 50000;
+
+    TestExternalitiesBuilder::<Test>::default()
+        .with_content_wg_config(
+            genesis::GenesisConfigBuilder::<Test>::default()
+                .with_mint_capacity(MINT_CAPACITY)
+                .build(),
+        )
+        .build()
+        .execute_with(|| {
+            let mint_id = ContentWorkingGroup::mint();
+            let mint = Minting::mints(mint_id);
+            assert_eq!(mint.capacity(), MINT_CAPACITY);
+
+            let increase = 25000;
+            // Increasing mint capacity
+            let expected_new_capacity = MINT_CAPACITY + increase;
+            assert_ok!(ContentWorkingGroup::increase_mint_capacity(
+                Origin::ROOT,
+                increase
+            ));
+            // Excpected event after increasing
+            assert_eq!(
+                get_last_event_or_panic(),
+                crate::RawEvent::MintCapacityIncreased(mint_id, increase, expected_new_capacity)
+            );
+            // Excpected value of capacity after increasing
+            let mint = Minting::mints(mint_id);
+            assert_eq!(mint.capacity(), expected_new_capacity);
+        });
+}
+
+#[test]
+fn setting_mint_capacity() {
+    const MINT_CAPACITY: u64 = 50000;
+
+    TestExternalitiesBuilder::<Test>::default()
+        .with_content_wg_config(
+            genesis::GenesisConfigBuilder::<Test>::default()
+                .with_mint_capacity(MINT_CAPACITY)
+                .build(),
+        )
+        .build()
+        .execute_with(|| {
+            let mint_id = ContentWorkingGroup::mint();
+            let mint = Minting::mints(mint_id);
+            assert_eq!(mint.capacity(), MINT_CAPACITY);
+
+            // Decreasing mint capacity
+            let new_lower_capacity = 10000;
+            let decrease = MINT_CAPACITY - new_lower_capacity;
+            assert_ok!(ContentWorkingGroup::set_mint_capacity(
+                Origin::ROOT,
+                new_lower_capacity
+            ));
+            // Correct event after decreasing
+            assert_eq!(
+                get_last_event_or_panic(),
+                crate::RawEvent::MintCapacityDecreased(mint_id, decrease, new_lower_capacity)
+            );
+            // Correct value of capacity after decreasing
+            let mint = Minting::mints(mint_id);
+            assert_eq!(mint.capacity(), new_lower_capacity);
+
+            // Increasing mint capacity
+            let new_higher_capacity = 25000;
+            let increase = new_higher_capacity - mint.capacity();
+            assert_ok!(ContentWorkingGroup::set_mint_capacity(
+                Origin::ROOT,
+                new_higher_capacity
+            ));
+            // Excpected event after increasing
+            assert_eq!(
+                get_last_event_or_panic(),
+                crate::RawEvent::MintCapacityIncreased(mint_id, increase, new_higher_capacity)
+            );
+            // Excpected value of capacity after increasing
+            let mint = Minting::mints(mint_id);
+            assert_eq!(mint.capacity(), new_higher_capacity);
+        });
+}

+ 82 - 48
runtime-modules/governance/src/election.rs

@@ -14,6 +14,7 @@ use super::sealed_vote::SealedVote;
 use super::stake::Stake;
 
 use super::council;
+use crate::election_params::ElectionParameters;
 pub use common::currency::{BalanceOf, GovernanceCurrency};
 
 pub trait Trait:
@@ -24,6 +25,8 @@ pub trait Trait:
     type CouncilElected: CouncilElected<Seats<Self::AccountId, BalanceOf<Self>>, Self::BlockNumber>;
 }
 
+pub static MSG_CANNOT_CHANGE_PARAMS_DURING_ELECTION: &str = "CannotChangeParamsDuringElection";
+
 #[derive(Clone, Copy, Encode, Decode)]
 pub enum ElectionStage<BlockNumber> {
     Announcing(BlockNumber),
@@ -106,8 +109,23 @@ decl_storage! {
         // TODO value type of this map looks scary, is there any way to simplify the notation?
         Votes get(votes): map T::Hash => SealedVote<T::AccountId, ElectionStake<T>, T::Hash, T::AccountId>;
 
-        // Current Election Parameters - default "zero" values are not meaningful. Running an election without
-        // settings reasonable values is a bad idea. Parameters can be set in the TriggerElection hook.
+        // Current Election Parameters.
+        // Should we replace all the individual values with a single ElectionParameters type?
+        // Having them individually makes it more flexible to add and remove new parameters in future
+        // without dealing with migration issues.
+
+        // We don't currently handle zero periods, zero council term, zero council size and candidacy
+        // limit in any special way. The behaviour in such cases:
+        // Setting any period to 0 will mean the election getting stuck in that stage, until force changing
+        // the state.
+        // Council Size of 0 - no limit to size of council, all applicants that move beyond
+        // announcing stage would become council members, so effectively the candidacy limit will
+        // be the size of the council, voting and revealing have no impact on final results.
+        // If candidacy limit is zero and council size > 0, council_size number of applicants will reach the voting stage.
+        // and become council members, voting will have no impact on final results.
+        // If both candidacy limit and council size are zero then all applicant become council members
+        // since no filtering occurs at end of announcing stage.
+        // We only guard against these edge cases in the set_election_parameters() call.
         AnnouncingPeriod get(announcing_period) config(): T::BlockNumber = T::BlockNumber::from(100);
         VotingPeriod get(voting_period) config(): T::BlockNumber = T::BlockNumber::from(100);
         RevealingPeriod get(revealing_period) config(): T::BlockNumber = T::BlockNumber::from(100);
@@ -195,7 +213,7 @@ impl<T: Trait> Module<T> {
 
         // Take snapshot of seat and backing stakes of an existing council
         // Its important to note that the election system takes ownership of these stakes, and is responsible
-        // to return any unused stake to original owners and the end of the election.
+        // to return any unused stake to original owners at the end of the election.
         Self::initialize_transferable_stakes(current_council);
 
         Self::deposit_event(RawEvent::ElectionStarted());
@@ -803,52 +821,19 @@ decl_module! {
             <Stage<T>>::put(ElectionStage::Voting(ends_at));
         }
 
-        fn set_param_announcing_period(origin, period: T::BlockNumber) {
-            ensure_root(origin)?;
-            ensure!(!Self::is_election_running(), "cannot change params during election");
-            ensure!(!period.is_zero(), "period cannot be zero");
-            <AnnouncingPeriod<T>>::put(period);
-        }
-        fn set_param_voting_period(origin,  period: T::BlockNumber) {
-            ensure_root(origin)?;
-            ensure!(!Self::is_election_running(), "cannot change params during election");
-            ensure!(!period.is_zero(), "period cannot be zero");
-            <VotingPeriod<T>>::put(period);
-        }
-        fn set_param_revealing_period(origin, period: T::BlockNumber) {
-            ensure_root(origin)?;
-            ensure!(!Self::is_election_running(), "cannot change params during election");
-            ensure!(!period.is_zero(), "period cannot be zero");
-            <RevealingPeriod<T>>::put(period);
-        }
-        fn set_param_min_council_stake(origin, amount: BalanceOf<T>) {
-            ensure_root(origin)?;
-            ensure!(!Self::is_election_running(), "cannot change params during election");
-            <MinCouncilStake<T>>::put(amount);
-        }
-        fn set_param_new_term_duration(origin, duration: T::BlockNumber) {
+        fn set_election_parameters(origin, params: ElectionParameters<BalanceOf<T>, T::BlockNumber>) {
             ensure_root(origin)?;
-            ensure!(!Self::is_election_running(), "cannot change params during election");
-            ensure!(!duration.is_zero(), "new term duration cannot be zero");
-            <NewTermDuration<T>>::put(duration);
-        }
-        fn set_param_council_size(origin, council_size: u32) {
-            ensure_root(origin)?;
-            ensure!(!Self::is_election_running(), "cannot change params during election");
-            ensure!(council_size > 0, "council size cannot be zero");
-            ensure!(council_size <= Self::candidacy_limit(), "council size cannot greater than candidacy limit");
-            CouncilSize::put(council_size);
-        }
-        fn set_param_candidacy_limit(origin, limit: u32) {
-            ensure_root(origin)?;
-            ensure!(!Self::is_election_running(), "cannot change params during election");
-            ensure!(limit >= Self::council_size(), "candidacy limit cannot be less than council size");
-            CandidacyLimit::put(limit);
-        }
-        fn set_param_min_voting_stake(origin, amount: BalanceOf<T>) {
-            ensure_root(origin)?;
-            ensure!(!Self::is_election_running(), "cannot change params during election");
-            <MinVotingStake<T>>::put(amount);
+            ensure!(!Self::is_election_running(), MSG_CANNOT_CHANGE_PARAMS_DURING_ELECTION);
+            params.ensure_valid()?;
+
+            <AnnouncingPeriod<T>>::put(params.announcing_period);
+            <VotingPeriod<T>>::put(params.voting_period);
+            <RevealingPeriod<T>>::put(params.revealing_period);
+            <MinCouncilStake<T>>::put(params.min_council_stake);
+            <NewTermDuration<T>>::put(params.new_term_duration);
+            CouncilSize::put(params.council_size);
+            CandidacyLimit::put(params.candidacy_limit);
+            <MinVotingStake<T>>::put(params.min_voting_stake);
         }
 
         fn force_stop_election(origin) {
@@ -2042,4 +2027,53 @@ mod tests {
             assert_ok!(Election::start_election(vec![]));
         });
     }
+
+    #[test]
+    fn setting_election_parameters() {
+        initial_test_ext().execute_with(|| {
+            let default_parameters: ElectionParameters<u64, u64> = ElectionParameters::default();
+            // default all zeros is invalid
+            assert!(default_parameters.ensure_valid().is_err());
+
+            let new_parameters = ElectionParameters {
+                announcing_period: 1,
+                voting_period: 2,
+                revealing_period: 3,
+                council_size: 4,
+                candidacy_limit: 5,
+                min_voting_stake: 6,
+                min_council_stake: 7,
+                new_term_duration: 8,
+            };
+
+            assert_ok!(Election::set_election_parameters(
+                Origin::ROOT,
+                new_parameters
+            ));
+
+            assert_eq!(
+                <AnnouncingPeriod<Test>>::get(),
+                new_parameters.announcing_period
+            );
+            assert_eq!(<VotingPeriod<Test>>::get(), new_parameters.voting_period);
+            assert_eq!(
+                <RevealingPeriod<Test>>::get(),
+                new_parameters.revealing_period
+            );
+            assert_eq!(
+                <MinCouncilStake<Test>>::get(),
+                new_parameters.min_council_stake
+            );
+            assert_eq!(
+                <NewTermDuration<Test>>::get(),
+                new_parameters.new_term_duration
+            );
+            assert_eq!(CouncilSize::get(), new_parameters.council_size);
+            assert_eq!(CandidacyLimit::get(), new_parameters.candidacy_limit);
+            assert_eq!(
+                <MinVotingStake<Test>>::get(),
+                new_parameters.min_voting_stake
+            );
+        });
+    }
 }

+ 45 - 0
runtime-modules/governance/src/election_params.rs

@@ -0,0 +1,45 @@
+use codec::{Decode, Encode};
+use sr_primitives::traits::Zero;
+use srml_support::{dispatch::Result, ensure};
+
+pub static MSG_PERIOD_CANNOT_BE_ZERO: &str = "PeriodCannotBeZero";
+pub static MSG_COUNCIL_SIZE_CANNOT_BE_ZERO: &str = "CouncilSizeCannotBeZero";
+pub static MSG_CANDIDACY_LIMIT_WAS_LOWER_THAN_COUNCIL_SIZE: &str =
+    "CandidacyWasLessThanCouncilSize";
+
+/// Combined Election parameters, as argument for set_election_parameters
+#[derive(Clone, Copy, Encode, Decode, Default, PartialEq, Debug)]
+pub struct ElectionParameters<Balance, BlockNumber> {
+    pub announcing_period: BlockNumber,
+    pub voting_period: BlockNumber,
+    pub revealing_period: BlockNumber,
+    pub council_size: u32,
+    pub candidacy_limit: u32,
+    pub new_term_duration: BlockNumber,
+    pub min_council_stake: Balance,
+    pub min_voting_stake: Balance,
+}
+
+impl<Balance, BlockNumber: PartialOrd + Zero> ElectionParameters<Balance, BlockNumber> {
+    pub fn ensure_valid(&self) -> Result {
+        self.ensure_periods_are_valid()?;
+        self.ensure_council_size_and_candidacy_limit_are_valid()?;
+        Ok(())
+    }
+
+    fn ensure_periods_are_valid(&self) -> Result {
+        ensure!(!self.announcing_period.is_zero(), MSG_PERIOD_CANNOT_BE_ZERO);
+        ensure!(!self.voting_period.is_zero(), MSG_PERIOD_CANNOT_BE_ZERO);
+        ensure!(!self.revealing_period.is_zero(), MSG_PERIOD_CANNOT_BE_ZERO);
+        Ok(())
+    }
+
+    fn ensure_council_size_and_candidacy_limit_are_valid(&self) -> Result {
+        ensure!(self.council_size > 0, MSG_COUNCIL_SIZE_CANNOT_BE_ZERO);
+        ensure!(
+            self.council_size <= self.candidacy_limit,
+            MSG_CANDIDACY_LIMIT_WAS_LOWER_THAN_COUNCIL_SIZE
+        );
+        Ok(())
+    }
+}

+ 1 - 0
runtime-modules/governance/src/lib.rs

@@ -3,6 +3,7 @@
 
 pub mod council;
 pub mod election;
+pub mod election_params;
 pub mod proposals;
 
 mod sealed_vote;

+ 9 - 0
runtime-modules/token-minting/src/lib.rs

@@ -73,6 +73,15 @@ impl From<MintingError> for TransferError {
     }
 }
 
+impl From<GeneralError> for &'static str {
+    fn from(err: GeneralError) -> &'static str {
+        match err {
+            GeneralError::MintNotFound => "MintNotFound",
+            GeneralError::NextAdjustmentInPast => "NextAdjustmentInPast",
+        }
+    }
+}
+
 #[derive(Encode, Decode, Copy, Clone, Debug, Eq, PartialEq)]
 pub enum Adjustment<Balance: Zero, BlockNumber> {
     // First adjustment will be after AdjustOnInterval.block_interval

+ 4 - 0
runtime/src/lib.rs

@@ -115,7 +115,11 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
     spec_name: create_runtime_str!("joystream-node"),
     impl_name: create_runtime_str!("joystream-node"),
     authoring_version: 6,
+<<<<<<< HEAD
     spec_version: 11,
+=======
+    spec_version: 10,
+>>>>>>> development
     impl_version: 0,
     apis: RUNTIME_API_VERSIONS,
 };