From bd186cc034a2b7fd3ac5c3288babdedd6b05d5f7 Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Fri, 18 Oct 2019 12:26:07 +0200 Subject: [PATCH 01/25] Initial crate setup --- .gitignore | 5 +++++ Cargo.toml | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 .gitignore create mode 100755 Cargo.toml diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000000..5515afb720 --- /dev/null +++ b/.gitignore @@ -0,0 +1,5 @@ +.DS_Store + +target + +Cargo.lock \ No newline at end of file diff --git a/Cargo.toml b/Cargo.toml new file mode 100755 index 0000000000..a0cd6d82b0 --- /dev/null +++ b/Cargo.toml @@ -0,0 +1,55 @@ +[package] +name = 'substrate-hiring-module' +version = '1.0.0' +authors = ['Bedeho Mender '] +edition = '2018' + +[dependencies] +hex-literal = '0.1.0' +serde = { version = '1.0', optional = true } +serde_derive = { version = '1.0', optional = true } +rstd = { package = 'sr-std', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} +runtime-primitives = { package = 'sr-primitives', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} +srml-support = { package = 'srml-support', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} +srml-support-procedural = { package = 'srml-support-procedural', git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} +system = { package = 'srml-system', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} +balances = { package = 'srml-balances', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} +codec = { package = 'parity-scale-codec', version = '1.0.0', default-features = false, features = ['derive'] } + +[dependencies.stake] +default_features = false +git = 'https://github.com/mnaamani/substrate-stake' +package = 'substrate-stake-module' +rev = 'eb4ef9711f1c375cc59f55b74b8a4e725335f4f0' # branch = 'staking' + +[dependencies.timestamp] +default_features = false +git = 'https://github.com/paritytech/substrate.git' +package = 'srml-timestamp' +rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b' + +[dependencies.runtime-io] +default_features = false +git = 'https://github.com/paritytech/substrate.git' +package = 'sr-io' +rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b' + +[dev-dependencies] +runtime-io = { package = 'sr-io', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} +primitives = { package = 'substrate-primitives', git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} + +[features] +default = ['std'] +std = [ + 'serde', + 'serde_derive', + 'codec/std', + 'rstd/std', + 'runtime-io/std', + 'runtime-primitives/std', + 'srml-support/std', + 'system/std', + 'balances/std', + 'timestamp/std', + 'stake/std' +] From d15f3f40f65e0cc8fc7a6ab9ca7a83e53156f576 Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Fri, 18 Oct 2019 12:32:41 +0200 Subject: [PATCH 02/25] Types, storage and module declaration --- src/hiring.rs | 285 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 285 insertions(+) create mode 100644 src/hiring.rs diff --git a/src/hiring.rs b/src/hiring.rs new file mode 100644 index 0000000000..df3af830f9 --- /dev/null +++ b/src/hiring.rs @@ -0,0 +1,285 @@ +use codec::{Decode, Encode}; +//use srml_support::traits::{Currency}; +//use runtime_primitives::traits::{SimpleArithmetic, Zero}; +//use srml_support::ensure; +//use rstd::collections::btree_map::BTreeMap; +use rstd::collections::btree_set::BTreeSet; + +//use crate::{Trait}; + +// Possible causes +#[derive(Encode, Decode, Debug, Eq, PartialEq, Clone, Copy, PartialOrd, Ord)] +pub enum ApplicationDeactivationCause { + External, // Add ID here for simplicity? + Hired, + NotHired, + CrowdedOut, + OpeningCancelled, + ReviewPeriodExpired, + OpeningFilled, +} + +/* +pub enum OutstandingUnstaking { + RoleOnly, + ApplicationOnly, + Both +} +*/ + +// Possible status of an application +#[derive(Encode, Decode, Debug, Eq, PartialEq, Clone, PartialOrd, Ord)] +pub enum ApplicationStage { + // Normal active state + Active, + + // Waiting for one or more unstakings, with a non-zero unstaking period, to complete. + Unstaking { + // When deactivation was initiated. + deactivation_initiated: BlockNumber, + + // The cause of the deactivation. + cause: ApplicationDeactivationCause, + }, + + // No longer active, can't do anything fun now. + Inactive { + // When deactivation was initiated. + deactivation_initiated: BlockNumber, + + // When deactivation was completed, and the inactive state was established. + deactivated: BlockNumber, + + // The cause of the deactivation. + cause: ApplicationDeactivationCause, + }, +} + +/// OpeningStage must be default constructible because it indirectly is a value in a storage map. +/// ***SHOULD NEVER ACTUALLY GET CALLED, IS REQUIRED TO DUE BAD STORAGE MODEL IN SUBSTRATE*** +impl Default for ApplicationStage { + fn default() -> Self { + ApplicationStage::Active + } +} + +// An application for an actor to occupy an opening. +#[derive(Encode, Decode, Default, Debug, Eq, PartialEq, Clone, PartialOrd, Ord)] +pub struct Application { + // Identifier for opening for which this application is for. + pub opening_id: OpeningId, + + // Index of arrival across all applications for given opening, + // which is needed for strictly ordering applications. + // Starts at 0. + pub application_index_in_opening: u32, + + // Block at which this application was added. + pub add_to_opening_in_block: BlockNumber, + + // NB: The given staking idnetifiers have a bloated purpose, + // and are mutable, fix this. + // https://github.com/Joystream/substrate-hiring-module/issues/11 + + // Identifier for stake that may possibly be established for role. + // Will be set iff the role staking policy of the corresponding opening + // states so AND application is not inactive. + pub active_role_staking_id: Option, + + // Identifier for stake that may possibly be established for application + // Will be set iff the application staking policy of the corresponding opening + // states so. + pub active_application_staking_id: Option, + + // Status of this application + pub stage: ApplicationStage, + + // ... + pub human_readable_text: Vec, +} + +// How to limit the number of eligible applicants +#[derive(Encode, Decode, Debug, Eq, PartialEq, Clone)] +pub struct ApplicationRationingPolicy { + // The maximum number of applications that can be on the list at any time. + pub max_active_applicants: u32, + // How applicants will be ranked, in order to respect the maximum simultaneous application limit + //pub applicant_ranking: ApplicationRankingPolicy +} + +#[derive(Encode, Decode, Debug, Eq, PartialEq, Clone)] +pub enum OpeningDeactivationCause { + CancelledBeforeActivation, + CancelledAcceptingApplications, + CancelledInReviewPeriod, + ReviewPeriodExpired, + Filled, +} + +#[derive(Encode, Decode, Debug, Eq, PartialEq, Clone)] +pub enum ActiveOpeningStage { + AcceptingApplications { + // + started_accepting_applicants_at_block: BlockNumber, + }, + + // + ReviewPeriod { + started_accepting_applicants_at_block: BlockNumber, + + started_review_period_at_block: BlockNumber, + }, + + // + Deactivated { + cause: OpeningDeactivationCause, + + deactivated_at_block: BlockNumber, + + started_accepting_applicants_at_block: BlockNumber, + + started_review_period_at_block: BlockNumber, + }, +} + +// The stage at which an `Opening` may be at. +#[derive(Encode, Decode, Debug, Eq, PartialEq, Clone)] +pub enum OpeningStage { + // .. + WaitingToBegin { + begins_at_block: BlockNumber, + }, + + // TODO: Fix this bad name + // + Active { + // Active stage + stage: ActiveOpeningStage, + + // Set of identifiers for all applications which have been added, but not removed, for this opening. + // Is required for timely on-chain lookup of all applications associated with an opening. + applicants: BTreeSet, //BTreeMap, //Vec, + + // TODO: Drop these counters + // https://github.com/Joystream/substrate-hiring-module/issues/9 + // + // Counters over all possible application states. + // Are needed to set `application_index_in_opening` in new applications + // Are very useful for light clients. + // + // NB: Remember that _all_ state transitions in applications will require updating these variables, + // its easy to forget! + // + // NB: The sum of + // - `active_application_count` + // - `unstaking_application_count` + // - `deactivated_application_count` + // + // equals the total number of applications ever added to the openig via `add_application`. + + // Active NOW + active_application_count: u32, + + // Unstaking NOW + unstaking_application_count: u32, + + // Deactivated at any time for any cause. + deactivated_application_count: u32, // Removed at any time. + //removed_application_count: u32 + }, +} + +impl OpeningStage { + /// The number of applications ever added to the opening via + /// `add_opening` extrinsic. + pub fn number_of_appliations_ever_added(&self) -> u32 { + match self { + OpeningStage::WaitingToBegin { .. } => 0, + + OpeningStage::Active { + active_application_count, + unstaking_application_count, + deactivated_application_count, + .. + } => { + active_application_count + + unstaking_application_count + + deactivated_application_count + } + } + } +} + +/// OpeningStage must be default constructible because it indirectly is a value in a storage map. +/// ***SHOULD NEVER ACTUALLY GET CALLED, IS REQUIRED TO DUE BAD STORAGE MODEL IN SUBSTRATE*** +impl Default for OpeningStage { + fn default() -> Self { + OpeningStage::WaitingToBegin { + begins_at_block: BlockNumber::default(), + } + } +} + +// Constraints around staking amount +#[derive(Encode, Decode, Debug, Eq, PartialEq, Clone)] +pub enum StakingAmountLimitMode { + AtLeast, + Exact, +} + +// Policy for staking +#[derive(Encode, Decode, Debug, Eq, PartialEq, Clone)] +pub struct StakingPolicy { + // Staking amount + pub amount: Balance, + + // How to interpret the amount requirement + pub amount_mode: StakingAmountLimitMode, + + // The unstaking period length, if any, deactivation causes that are autonomous, + // that is they are triggered internally to this module. + pub crowded_out_unstaking_period_length: Option, + pub review_period_expired_unstaking_period_length: Option, +} + +/* +impl StakingPolicy { + + pub fn amount_is_valid(&self) -> bool { + self.amount >= T::Currency::minimum_balance() + } +} +*/ + +impl StakingPolicy { + pub fn accepts_amount(&self, test_amount: &Balance) -> bool { + match self.amount_mode { + StakingAmountLimitMode::AtLeast => *test_amount >= self.amount, + StakingAmountLimitMode::Exact => *test_amount == self.amount, + } + } +} + +#[derive(Encode, Decode, Default, Debug, Eq, PartialEq, Clone)] +pub struct Opening { + // Block at which opening was added + pub created: BlockNumber, + + // Current stage for this opening + pub stage: OpeningStage, + + // Maximum length of the review stage. + pub max_review_period_length: BlockNumber, + + // Whether, and if so how, to limit the number of active applicants.... + pub application_rationing_policy: Option, + + // Whether any staking is required just to apply, and if so, how that stake is managed. + pub application_staking_policy: Option>, + + // Whether any staking is required for the role, and if so, how that stake is managed. + pub role_staking_policy: Option>, + + // Description of opening + pub human_readable_text: Vec, +} From cc6a75c9aaed5e05d861b0e7d33d025101937bfc Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Fri, 18 Oct 2019 12:33:18 +0200 Subject: [PATCH 03/25] Public methods --- src/lib.rs | 1610 ++++++++++++++++++++++++++++++++++++++++++++++++ src/macroes.rs | 319 ++++++++++ 2 files changed, 1929 insertions(+) create mode 100755 src/lib.rs create mode 100644 src/macroes.rs diff --git a/src/lib.rs b/src/lib.rs new file mode 100755 index 0000000000..6daa6c9ef0 --- /dev/null +++ b/src/lib.rs @@ -0,0 +1,1610 @@ +// Ensure we're `no_std` when compiling for Wasm. +#![cfg_attr(not(feature = "std"), no_std)] + +#[cfg(feature = "std")] +//use serde_derive::{Deserialize, Serialize}; +use rstd::prelude::*; + +use codec::{Codec, Decode, Encode}; +use runtime_primitives::traits::{MaybeSerializeDebug, Member, One, SimpleArithmetic}; +use srml_support::traits::Currency; +use srml_support::{ + decl_module, decl_storage, ensure, EnumerableStorageMap, Parameter, StorageMap, StorageValue, +}; +use std::iter::Iterator; + +use runtime_primitives::traits::Zero; + +use crate::sr_api_hidden_includes_decl_storage::hidden_include::traits::Imbalance; + +use rstd::collections::btree_map::BTreeMap; +use rstd::collections::btree_set::BTreeSet; + +mod hiring; +mod macroes; +mod mock; +mod test; + +use hiring::*; +use system; + +use stake; + +/// ... +pub trait ApplicationDeactivatedHandler { + /// An application, with the given id, was fully deactivated, with the + /// given cause, and was put in the inactive state. + fn deactivated(application_id: &T::ApplicationId, cause: &hiring::ApplicationDeactivationCause); +} + +pub trait Trait: system::Trait + stake::Trait + Sized { + type OpeningId: Parameter + + Member + + SimpleArithmetic + + Codec + + Default + + Copy + + MaybeSerializeDebug + + PartialEq; + + type ApplicationId: Parameter + + Member + + SimpleArithmetic + + Codec + + Default + + Copy + + MaybeSerializeDebug + + PartialEq; + + /// Type that will handle various staking events + type ApplicationDeactivatedHandler: ApplicationDeactivatedHandler; +} + +/// Helper implementation so we can provide multiple handlers by grouping handlers in tuple pairs. +/// For example for three handlers, A, B and C we can set the StakingEventHandler type on the trait to: +/// type StakingEventHandler = ((A, B), C) +/// +impl ApplicationDeactivatedHandler for () { + fn deactivated( + _application_id: &T::ApplicationId, + _cause: &hiring::ApplicationDeactivationCause, + ) { + } +} + +/* +impl, Y: ApplicationDeactivatedHandler> ApplicationDeactivatedHandler + for (X, Y) +{ + fn deactivated( + application_id: T::ApplicationId, + cause: hiring::ApplicationDeactivationCause + ) { + X::deactivated(application_id, cause); + Y::deactivated(application_id, cause); + } +} +*/ + +pub type BalanceOf = + <::Currency as Currency<::AccountId>>::Balance; + +pub type NegativeImbalance = + <::Currency as Currency<::AccountId>>::NegativeImbalance; + +decl_storage! { + trait Store for Module as Hiring { + + /// Openings. + pub OpeningById get(opening_by_id): linked_map T::OpeningId => Opening, T::BlockNumber, T::ApplicationId>; + + /// Identifier for next opening to be added. + pub NextOpeningId get(next_opening_id): T::OpeningId; + + /// Applications + pub ApplicationById get(application_by_id): linked_map T::ApplicationId => Application; + + /// Identifier for next application to be added. + pub NextApplicationId get(next_application_id): T::ApplicationId; + + /// Internal purpose of given stake, i.e. fro what application, and whether for the role or for the application. + pub ApplicationIdByStakingId get(stake_purpose_by_staking_id): linked_map T::StakeId => T::ApplicationId; + + } +} + +decl_module! { + pub struct Module for enum Call where origin: T::Origin { + + fn on_finalize(now: T::BlockNumber) { + + // + // == MUTATION SAFE == + // + + // NB: This routine is plauged by this problem + // https://github.com/Joystream/joystream/issues/126#issuecomment-542268343 , + // would be much cleaner otherwise. + + // Compute iterator of openings waiting to begin + let openings_ready_to_accept_applications_iter = + >::enumerate() + .filter_map(|(opening_id, opening)| { + + if let hiring::OpeningStage::WaitingToBegin { + begins_at_block + } = opening.stage { + + if begins_at_block == now { + Some(( + opening_id, + opening, + begins_at_block + )) + } else { + None + } + } else { + None + } + }); + + // ... + for (opening_id, opening, _) in openings_ready_to_accept_applications_iter { + + // NB: Shows issue https://github.com/Joystream/substrate-hiring-module/issues/5 + let opening_accepting_applications = hiring::Opening{ + stage : hiring::OpeningStage::Active { + stage: hiring::ActiveOpeningStage::AcceptingApplications { + started_accepting_applicants_at_block: now + }, + applicants: BTreeSet::new(), + active_application_count: 0, + unstaking_application_count: 0, + deactivated_application_count: 0 + }, + ..(opening.clone()) + }; + + >::insert(opening_id, opening_accepting_applications); + } + + // Compute iterator of openings in expired review period + let openings_in_expired_review_period_iter = + >::enumerate() + .filter_map(|(opening_id, opening)| { + + if let hiring::OpeningStage::Active { + ref stage, + ref applicants, + ref active_application_count, + ref unstaking_application_count, + ref deactivated_application_count + } = opening.stage { + + if let hiring::ActiveOpeningStage::ReviewPeriod { + ref started_accepting_applicants_at_block, + ref started_review_period_at_block + } = stage { + + Some(( + opening_id, + opening.clone(), + ( + stage.clone(), + applicants.clone(), + *active_application_count, + *unstaking_application_count, + *deactivated_application_count, + started_accepting_applicants_at_block.clone(), + started_review_period_at_block.clone() + ) + )) + + } else { + None + } + } else { + None + } + }); + + // ... + for (opening_id, + opening, + ( + _stage, + applicants, + _active_application_count, + _unstaking_application_count, + _deactivated_application_count, + started_accepting_applicants_at_block, + started_review_period_at_block + )) in openings_in_expired_review_period_iter { + + // + // Deactivat all applications that are part of this opening + // + + // Get unstaking periods + let application_stake_unstaking_period = Self::opt_staking_policy_to_review_period_expired_unstaking_period(&opening.application_staking_policy); + let role_stake_unstaking_period = Self::opt_staking_policy_to_review_period_expired_unstaking_period(&opening.role_staking_policy); + + + // Get applications + let applications_map = Self::application_id_iter_to_map(applicants.iter()); + + // Deactivate + Self::initiate_application_deactivations( + &applications_map, + application_stake_unstaking_period, + role_stake_unstaking_period, + &hiring::ApplicationDeactivationCause::ReviewPeriodExpired + ); + + // NB: Shows issue https://github.com/Joystream/substrate-hiring-module/issues/5 + let opening_accepting_applications = hiring::Opening { + + stage: hiring::OpeningStage::Active { + + stage: hiring::ActiveOpeningStage::Deactivated { + + cause: hiring::OpeningDeactivationCause::ReviewPeriodExpired, + deactivated_at_block: now, + started_accepting_applicants_at_block: started_accepting_applicants_at_block, + started_review_period_at_block: started_review_period_at_block, + }, + + applicants: BTreeSet::new(), + active_application_count: 0, + unstaking_application_count: 0, + deactivated_application_count: 0 + }, + + ..(opening.clone()) + }; + + >::insert(opening_id, opening_accepting_applications); + + } + + } + } +} + +#[derive(Debug, PartialEq)] +pub enum StakePurpose { + Role, + Application, +} + +// Safe and explict way of chosing +#[derive(Encode, Decode, Debug, Eq, PartialEq)] +pub enum ActivateOpeningAt { + CurrentBlock, + ExactBlock(T::BlockNumber), +} + +#[derive(Debug, PartialEq)] +pub enum AddOpeningError { + OpeningMustActivateInTheFuture, + + /// It is not possible to stake less than the minimum balance defined in the + /// `Currency` module. + StakeAmountLessThanMinimumCurrencyBalance(StakePurpose), +} + +/* +pub struct ApplicationUnstakingPeriods { + successful_applicants: T::BlockNumber, + unsuccessful_applicants: T::BlockNumber +} +*/ + +/// The possible outcome for an application in an opening which is being filled. +pub enum ApplicationOutcomeInFilledOpening { + Success, + Failure, +} + +/// Error due to attempting to fill an opening. +pub enum FillOpeningError { + OpeningDoesNotExist, + OpeningNotInReviewPeriodStage, + UnstakingPeriodTooShort(StakePurpose, ApplicationOutcomeInFilledOpening), + RedundantUnstakingPeriodProvided(StakePurpose, ApplicationOutcomeInFilledOpening), + ApplicationDoesNotExist(T::ApplicationId), + ApplicationNotInActiveStage(T::ApplicationId), +} + +pub enum CancelOpeningError { + UnstakingPeriodTooShort(StakePurpose), + RedundantUnstakingPeriodProvided(StakePurpose), + OpeningDoesNotExist, + OpeningNotInCancellableStage, +} + +/// NB: +/// `OpeningCancelled` does not have the ideal form. +/// https://github.com/Joystream/substrate-hiring-module/issues/10 +pub struct OpeningCancelled { + pub number_of_unstaking_applications: u32, + pub number_of_deactivated_applications: u32, +} + +pub enum RemoveOpeningError { + OpeningDoesNotExist, +} + +pub enum BeginReviewError { + OpeningDoesNotExist, + OpeningNotInAcceptingApplicationsStage, +} + +pub enum BeginAcceptingApplicationsError { + OpeningDoesNotExist, + OpeningIsNotInWaitingToBeginStage, +} + +pub enum AddApplicationError { + OpeningDoesNotExist, + StakeProvidedWhenRedundant(StakePurpose), + StakeMissingWhenRequired(StakePurpose), + StakeAmountTooLow(StakePurpose), + OpeningNotInAcceptingApplicationsStage, + NewApplicationWasCrowdedOut, +} + +pub enum ApplicationAdded { + NoCrowdingOut, + CrodedOutApplication(ApplicationId), +} + +pub enum DeactivateApplicationError { + ApplicationDoesNotExist, + ApplicationNotActive, + OpeningNotAcceptingApplications, + UnstakingPeriodTooShort(StakePurpose), + RedundantUnstakingPeriodProvided(StakePurpose), +} + +impl Module { + pub fn add_opening( + activate_at: ActivateOpeningAt, + max_review_period_length: T::BlockNumber, + application_rationing_policy: Option, + application_staking_policy: Option, T::BlockNumber>>, + role_staking_policy: Option, T::BlockNumber>>, + human_readable_text: Vec, + ) -> Result { + let current_block_height = >::block_number(); + + // Check that exact activation is actually in the future + ensure!( + match activate_at { + ActivateOpeningAt::ExactBlock(block_number) => block_number > current_block_height, + _ => true, + }, + AddOpeningError::OpeningMustActivateInTheFuture + ); + + // Check that staking amounts clear minimum balance required. + ensure_amount_valid_in_opt_staking_policy!( + T, + application_staking_policy, + AddOpeningError::StakeAmountLessThanMinimumCurrencyBalance(StakePurpose::Application) + )?; + + ensure_amount_valid_in_opt_staking_policy!( + T, + role_staking_policy, + AddOpeningError::StakeAmountLessThanMinimumCurrencyBalance(StakePurpose::Role) + )?; + + // + // == MUTATION SAFE == + // + + // Construct new opening + let opening_stage = match activate_at { + ActivateOpeningAt::CurrentBlock => hiring::OpeningStage::Active { + // We immediately start accepting applications + stage: hiring::ActiveOpeningStage::AcceptingApplications { + started_accepting_applicants_at_block: current_block_height, + }, + + // Empty set of applicants + applicants: BTreeSet::new(), // Map::new(), + + // All counters set to 0 + active_application_count: 0, + unstaking_application_count: 0, + deactivated_application_count: 0, + }, + + ActivateOpeningAt::ExactBlock(block_number) => hiring::OpeningStage::WaitingToBegin { + begins_at_block: block_number, + }, + }; + + let new_opening = hiring::Opening { + created: current_block_height, + stage: opening_stage, + max_review_period_length: max_review_period_length, + application_rationing_policy: application_rationing_policy, + application_staking_policy: application_staking_policy, + role_staking_policy: role_staking_policy, + human_readable_text: human_readable_text, + }; + + // Get Id for new opening + let new_opening_id = >::get(); + + // Insert opening in storage + >::insert(new_opening_id, new_opening); + + // Update NextOpeningId counter + >::mutate(|id| *id += T::OpeningId::from(1)); + + // Return + Ok(new_opening_id) + } + + /// Cancels opening with given identifier, using provided unstaking periods for + /// application and role, as necesary. + pub fn cancel_opening( + opening_id: T::OpeningId, + application_stake_unstaking_period: Option, + role_stake_unstaking_period: Option, + ) -> Result { + // Ensure that the opening exists + let opening = + ensure_opening_exists!(T, opening_id, CancelOpeningError::OpeningDoesNotExist)?; + + // Opening is in stage Active.{AcceptingApplications or ReviewPeriod} + + let ( + active_stage, + applicants, + _active_application_count, + _unstaking_application_count, + _deactivated_application_count, + ) = ensure_opening_is_active!( + opening.stage, + CancelOpeningError::OpeningNotInCancellableStage + )?; + + // NB: Not macroised yet, since this is only place where its used. + ensure!( + match active_stage { + ActiveOpeningStage::AcceptingApplications { .. } + | ActiveOpeningStage::ReviewPeriod { .. } => true, + _ => false, + }, + CancelOpeningError::OpeningNotInCancellableStage + ); + + // Ensure unstaking periods are OK. + ensure_opt_unstaking_period_is_ok!( + application_stake_unstaking_period, + opening.application_staking_policy, + CancelOpeningError::UnstakingPeriodTooShort(StakePurpose::Application), + CancelOpeningError::RedundantUnstakingPeriodProvided(StakePurpose::Application) + )?; + + ensure_opt_unstaking_period_is_ok!( + role_stake_unstaking_period, + opening.role_staking_policy, + CancelOpeningError::UnstakingPeriodTooShort(StakePurpose::Role), + CancelOpeningError::RedundantUnstakingPeriodProvided(StakePurpose::Role) + )?; + + // + // == MUTATION SAFE == + // + + // Map with applications + let applications_map = Self::application_id_iter_to_map(applicants.iter()); + + // Initiate deactivation of all active applications + let net_result = Self::initiate_application_deactivations( + &applications_map, + application_stake_unstaking_period, + role_stake_unstaking_period, + &hiring::ApplicationDeactivationCause::OpeningCancelled, + ); + + // Return + Ok(OpeningCancelled { + number_of_unstaking_applications: net_result.number_of_unstaking_applications, + number_of_deactivated_applications: net_result.number_of_deactivated_applications, + }) + } + + /* + pub fn remove_opening( + opening_id: T::OpeningId + ) -> Result { + + // Applies when a given opening is in stage WaitingToBegin or Began.Inactive. + // In the latter it is also required that all applications are inactive. + // Opening, and all associated applications, are removed from openingsById and applicationsById. + // The number of applications removed is returned. + // + + Ok(0) + } + */ + + pub fn begin_accepting_applications( + opening_id: T::OpeningId, + ) -> Result<(), BeginAcceptingApplicationsError> { + // Ensure that the opening exists + let opening = ensure_opening_exists!( + T, + opening_id, + BeginAcceptingApplicationsError::OpeningDoesNotExist + )?; + + // Ensure that it is the waiting to begin stage + ensure_opening_stage_is_waiting_to_begin!( + opening.stage, + BeginAcceptingApplicationsError::OpeningIsNotInWaitingToBeginStage + )?; + + // + // == MUTATION SAFE == + // + + let current_block_height = >::block_number(); + + // Update state of opening + let new_opening = hiring::Opening { + stage: hiring::OpeningStage::Active { + stage: hiring::ActiveOpeningStage::AcceptingApplications { + started_accepting_applicants_at_block: current_block_height, + }, + applicants: BTreeSet::new(), //BTreeMap::new(), + active_application_count: 0, + unstaking_application_count: 0, + deactivated_application_count: 0, + }, + ..opening + }; + + // Write back opening + >::insert(opening_id, new_opening); + + // DONE + Ok(()) + } + + pub fn begin_review(opening_id: T::OpeningId) -> Result<(), BeginReviewError> { + // Ensure that the opening exists + let opening = ensure_opening_exists!(T, opening_id, BeginReviewError::OpeningDoesNotExist)?; + + // Opening is accepting applications + + let ( + active_stage, + applicants, + active_application_count, + unstaking_application_count, + deactivated_application_count, + ) = ensure_opening_is_active!( + opening.stage, + BeginReviewError::OpeningNotInAcceptingApplicationsStage + )?; + + let started_accepting_applicants_at_block = ensure_active_opening_is_accepting_applications!( + active_stage, + BeginReviewError::OpeningNotInAcceptingApplicationsStage + )?; + + // + // == MUTATION SAFE == + // + + let current_block_height = >::block_number(); + + let new_opening = hiring::Opening { + stage: hiring::OpeningStage::Active { + stage: hiring::ActiveOpeningStage::ReviewPeriod { + started_accepting_applicants_at_block: started_accepting_applicants_at_block, + started_review_period_at_block: current_block_height, + }, + applicants, + active_application_count, + unstaking_application_count, + deactivated_application_count, + }, + ..opening + }; + + // Update to new opening + >::insert(opening_id, new_opening); + + Ok(()) + } + + /// Fill an opening, identifed with `opening_id`, currently in the review period. + /// + /// TODO: Name _a bit_ too long? but descriptive though... + pub fn fill_opening( + opening_id: T::OpeningId, + successful_applications: BTreeSet, + opt_successful_applicant_application_stake_unstaking_period: Option, + opt_failed_applicant_application_stake_unstaking_period: Option, + /* this parameter does not make sense? opt_successful_applicant_role_stake_unstaking_period: Option, */ + opt_failed_applicant_role_stake_unstaking_period: Option, + ) -> Result<(), FillOpeningError> { + // Ensure that the opening exists + let opening = ensure_opening_exists!(T, opening_id, FillOpeningError::OpeningDoesNotExist)?; + + let ( + active_stage, + applicants, + active_application_count, + unstaking_application_count, + deactivated_application_count, + ) = ensure_opening_is_active!( + opening.stage, + FillOpeningError::OpeningNotInReviewPeriodStage + )?; + + // Ensure opening is in review period + let (started_accepting_applicants_at_block, started_review_period_at_block) = ensure_active_opening_is_in_review_period!( + active_stage, + FillOpeningError::OpeningNotInReviewPeriodStage + )?; + + // + // Ensure that all unstaking periods are neither too short (0) nor redundant. + // + + ensure_opt_unstaking_period_is_ok!( + opt_successful_applicant_application_stake_unstaking_period, + opening.application_staking_policy, + FillOpeningError::UnstakingPeriodTooShort( + StakePurpose::Application, + ApplicationOutcomeInFilledOpening::Success + ), + FillOpeningError::RedundantUnstakingPeriodProvided( + StakePurpose::Application, + ApplicationOutcomeInFilledOpening::Success + ) + )?; + + ensure_opt_unstaking_period_is_ok!( + opt_failed_applicant_application_stake_unstaking_period, + opening.application_staking_policy, + FillOpeningError::UnstakingPeriodTooShort( + StakePurpose::Application, + ApplicationOutcomeInFilledOpening::Failure + ), + FillOpeningError::RedundantUnstakingPeriodProvided( + StakePurpose::Application, + ApplicationOutcomeInFilledOpening::Failure + ) + )?; + + /* + ensure_opt_unstaking_period_is_ok!( + opt_successful_applicant_role_stake_unstaking_period, + opening.role_staking_policy, + FillOpeningError::UnstakingPeriodTooShort(StakePurpose::Application, ApplicationOutcomeInFilledOpening::Success), + FillOpeningError::RedundantUnstakingPeriodProvided(StakePurpose::Application, ApplicationOutcomeInFilledOpening::Success) + )?; + */ + + ensure_opt_unstaking_period_is_ok!( + opt_failed_applicant_role_stake_unstaking_period, + opening.role_staking_policy, + FillOpeningError::UnstakingPeriodTooShort( + StakePurpose::Application, + ApplicationOutcomeInFilledOpening::Failure + ), + FillOpeningError::RedundantUnstakingPeriodProvided( + StakePurpose::Application, + ApplicationOutcomeInFilledOpening::Failure + ) + )?; + + // Ensure that all provided application ids are infact valid + let invalid_application_ids = successful_applications + .clone() + .iter() + .filter_map(|application_id| { + if !>::exists(application_id) { + Some(*application_id) + } else { + None + } + }) + .collect::>(); + + if !invalid_application_ids.is_empty() { + let first_missing_application_id = invalid_application_ids.iter().next().unwrap(); + + return Err(FillOpeningError::ApplicationDoesNotExist( + *first_missing_application_id, + )); + } + + // Ensure that all claimed successful applications actually exist, and collect@ + // underlying applications into a map. + let successful_applications_map = successful_applications + .clone() + .iter() + .map(|application_id| { + assert!(>::exists(application_id)); + + let application = >::get(application_id); + + (*application_id, application) + }) + .collect::>(); + + // Ensure that all successful applications are actually active. + let opt_non_active_application = successful_applications_map + .iter() + .find(|(_application_id, application)| application.stage != ApplicationStage::Active); + + if let Some((application_id, _application)) = opt_non_active_application { + return Err(FillOpeningError::ApplicationNotInActiveStage( + *application_id, + )); + } + + // + // == MUTATION SAFE == + // + + // Deactivate all successful applications, with cause being hired + for (application_id, application) in &successful_applications_map { + Self::try_to_initiate_application_deactivation( + &application, + *application_id, + opt_successful_applicant_application_stake_unstaking_period, + None, // <= We do not unstake role stake for successful applicants, opt_successful_applicant_role_stake_unstaking_period, + &hiring::ApplicationDeactivationCause::Hired, + ); + } + + // Deactivate all unsuccessful applications, with cause being not being hired. + + // First get all failed applications by their id. + let failed_applications_map = applicants + .difference(&successful_applications) + .cloned() + .map(|application_id| { + let application = >::get(application_id); + + (application_id, application) + }) + .collect::>(); + + // Deactivate all successful applications, with cause being hired + for (application_id, application) in &failed_applications_map { + Self::try_to_initiate_application_deactivation( + &application, + *application_id, + opt_failed_applicant_application_stake_unstaking_period, + opt_failed_applicant_role_stake_unstaking_period, + &hiring::ApplicationDeactivationCause::NotHired, + ); + } + + // Grab current block height + let current_block_height = >::block_number(); + + // Deactivate opening itself + let new_opening = hiring::Opening { + stage: hiring::OpeningStage::Active { + stage: hiring::ActiveOpeningStage::Deactivated { + cause: OpeningDeactivationCause::Filled, + deactivated_at_block: current_block_height, + started_accepting_applicants_at_block: started_accepting_applicants_at_block, + started_review_period_at_block: started_review_period_at_block, + }, + //.. <== cant use here, same issue + applicants: applicants, + active_application_count: active_application_count, + unstaking_application_count: unstaking_application_count, + deactivated_application_count: deactivated_application_count, + }, + ..opening + }; + + // Write back new opening + >::insert(opening_id, new_opening); + + // DONE + Ok(()) + } + + /// Adds a new application on the given opening, and begins staking for + /// the role, the application or both possibly. + /// + /// Returns .. + pub fn add_application( + opening_id: T::OpeningId, + opt_role_stake_imbalance: Option>, + opt_application_stake_imbalance: Option>, + human_readable_text: Vec, + ) -> Result, AddApplicationError> { + // NB: Should we provide the two imbalances back?? + + // Ensure that the opening exists + let opening = + ensure_opening_exists!(T, opening_id, AddApplicationError::OpeningDoesNotExist)?; + + // Ensure that proposed stakes match the policy of the opening. + let opt_role_stake_balance = ensure_stake_imbalance_matches_staking_policy!( + &opt_role_stake_imbalance, + &opening.role_staking_policy, + AddApplicationError::StakeMissingWhenRequired(StakePurpose::Role), + AddApplicationError::StakeProvidedWhenRedundant(StakePurpose::Role), + AddApplicationError::StakeAmountTooLow(StakePurpose::Role) + )?; + + let opt_application_stake_balance = ensure_stake_imbalance_matches_staking_policy!( + &opt_application_stake_imbalance, + &opening.application_staking_policy, + AddApplicationError::StakeMissingWhenRequired(StakePurpose::Application), + AddApplicationError::StakeProvidedWhenRedundant(StakePurpose::Application), + AddApplicationError::StakeAmountTooLow(StakePurpose::Application) + )?; + + // Opening is accepting applications + + let ( + active_stage, + applicants, + active_application_count, + unstaking_application_count, + deactivated_application_count, + ) = ensure_opening_is_active!( + opening.stage, + AddApplicationError::OpeningNotInAcceptingApplicationsStage + )?; + + ensure_active_opening_is_accepting_applications!( + active_stage, + AddApplicationError::OpeningNotInAcceptingApplicationsStage + )?; + + // Ensure that the new application would actually make it + let success = ensure_application_would_get_added!( + &opening.application_rationing_policy, + &applicants, + &opt_role_stake_balance, + &opt_application_stake_balance, + AddApplicationError::NewApplicationWasCrowdedOut + )?; + + // + // == MUTATION SAFE == + // + + // If required, deactive another application that was crowded out. + if let ApplicationAddedSuccess::CrowdsOutExistingApplication( + id_of_croweded_out_application, + ) = success + { + // Get relevant unstaking periods + let opt_application_stake_unstaking_period = + Self::opt_staking_policy_to_crowded_out_unstaking_period( + &opening.application_staking_policy, + ); + let opt_role_stake_unstaking_period = + Self::opt_staking_policy_to_crowded_out_unstaking_period( + &opening.role_staking_policy, + ); + + // Fetch application + let crowded_out_application = >::get(id_of_croweded_out_application); + + // Initiate actual deactivation + // + // MUST not have been ignored, is runtime invariant, false means code is broken. + // But should we do panic in runtime? Is there safer way? + assert_ne!( + Self::try_to_initiate_application_deactivation( + &crowded_out_application, + id_of_croweded_out_application, + opt_application_stake_unstaking_period, + opt_role_stake_unstaking_period, + &hiring::ApplicationDeactivationCause::CrowdedOut + ), + ApplicationDeactivationInitationResult::Ignored + ); + } + + // Get Id for this new application + let new_application_id = >::get(); + + // Possibly initiate staking + let active_role_staking_id = + Self::infallible_opt_stake_initiation(opt_role_stake_imbalance, &new_application_id); + let active_application_staking_id = Self::infallible_opt_stake_initiation( + opt_application_stake_imbalance, + &new_application_id, + ); + + // Stage of new application + let application_stage = hiring::ApplicationStage::Active; + + // Grab current block height + let current_block_height = >::block_number(); + + // Compute index for this new application + // TODO: fix so that `number_of_appliations_ever_added` can be invoked. + let application_index_in_opening = + active_application_count + unstaking_application_count + deactivated_application_count; // cant do this due to bad design of stage => opening.stage.number_of_appliations_ever_added(); + + // Create a new application + let new_application = hiring::Application { + opening_id: opening_id, + application_index_in_opening: application_index_in_opening, + add_to_opening_in_block: current_block_height, + active_role_staking_id: active_role_staking_id, + active_application_staking_id: active_application_staking_id, + stage: application_stage, + human_readable_text: human_readable_text, + }; + + // Insert into main application map + >::insert(new_application_id, new_application); + + // Update next application id + >::mutate(|id| *id += One::one()); + + // Update counter on opening + >::mutate(opening_id, |opening| { + /* + TODO: + Yet another instance of problems due to not following https://github.com/Joystream/joystream/issues/36#issuecomment-539567407 + */ + opening.stage = hiring::OpeningStage::Active { + stage: active_stage, + applicants: applicants, + active_application_count: active_application_count + 1, + unstaking_application_count: unstaking_application_count, + deactivated_application_count: deactivated_application_count, + }; + }); + + // DONE + Ok(match success { + ApplicationAddedSuccess::CrowdsOutExistingApplication( + id_of_croweded_out_application, + ) => ApplicationAdded::CrodedOutApplication(id_of_croweded_out_application), + _ => ApplicationAdded::NoCrowdingOut, + }) + } + + /// Deactive an active application. + /// + /// Does currently not support slashing + pub fn deactive_application( + application_id: T::ApplicationId, + application_stake_unstaking_period: Option, + role_stake_unstaking_period: Option, + ) -> Result<(), DeactivateApplicationError> { + // Check that application id is valid, and if so, + // grab corresponding application and opening. + let (application, opening) = ensure_application_exists!( + T, + application_id, + DeactivateApplicationError::ApplicationDoesNotExist, + auto_fetch_opening + )?; + + // Application is active + ensure_eq!( + application.stage, + hiring::ApplicationStage::Active, + DeactivateApplicationError::ApplicationNotActive + ); + + // Opening is accepting applications + let (active_stage, ..) = ensure_opening_is_active!( + opening.stage, + DeactivateApplicationError::OpeningNotAcceptingApplications + )?; + + ensure_active_opening_is_accepting_applications!( + active_stage, + DeactivateApplicationError::OpeningNotAcceptingApplications + )?; + + // Ensure unstaking periods are OK. + ensure_opt_unstaking_period_is_ok!( + application_stake_unstaking_period, + opening.application_staking_policy, + DeactivateApplicationError::UnstakingPeriodTooShort(StakePurpose::Application), + DeactivateApplicationError::RedundantUnstakingPeriodProvided(StakePurpose::Application) + )?; + + ensure_opt_unstaking_period_is_ok!( + role_stake_unstaking_period, + opening.role_staking_policy, + DeactivateApplicationError::UnstakingPeriodTooShort(StakePurpose::Role), + DeactivateApplicationError::RedundantUnstakingPeriodProvided(StakePurpose::Role) + )?; + + // + // == MUTATION SAFE == + // + + // Deactive application + let result = Self::try_to_initiate_application_deactivation( + &application, + application_id, + application_stake_unstaking_period, + role_stake_unstaking_period, + &hiring::ApplicationDeactivationCause::External, + ); + + // MUST not have been ignored, is runtime invariant, false means code is broken. + // But should we do panic in runtime? Is there safer way? + assert_ne!(result, ApplicationDeactivationInitationResult::Ignored); + + // DONE + Ok(()) + } + + /* + pub fn remove_application() ->() { + + // Applies when an application is in the stage Inactive. Results in removing instance from applicationsById, + // and from the corresponding opening. Returns nothing. + } + */ + + /// The stake, with the given id, was unstaked. + pub fn unstaked(stake_id: T::StakeId) { + // Ignore unstaked + if !>::exists(stake_id) { + return; + } + + // Get application + let application_id = >::get(stake_id); + + assert!(>::exists(application_id)); + + let application = >::get(application_id); + + // Make sure that we are actually unstaking, ignore otherwise. + let (deactivation_initiated, cause) = if let ApplicationStage::Unstaking { + deactivation_initiated, + cause, + } = application.stage + { + (deactivation_initiated, cause) + } else { + return; + }; + + // + // == MUTATION SAFE == + // + + // Drop stake from stake to application map + >::remove(stake_id); + + // New values for application stakes + let new_active_role_staking_id = match application.active_role_staking_id { + // If there is a match, reset. + Some(id) => { + if id == stake_id { + None + } else { + Some(id) + } + } + _ => None, + }; + + let new_active_application_staking_id = match application.active_application_staking_id { + // If there is a match, reset. + Some(id) => { + if id == stake_id { + None + } else { + Some(id) + } + } + _ => None, + }; + + // Are we now done unstaking? + // Is the case if thereare no application stakes set. + let is_now_done_unstaking = + new_active_role_staking_id.is_none() && new_active_application_staking_id.is_none(); + + // Stage of application after unstaking. + let new_stage = + + // If we are done unstaking, then we go to the inactive stage + if is_now_done_unstaking { + + let current_block_height = >::block_number(); + + ApplicationStage::Inactive { + deactivation_initiated: deactivation_initiated, + deactivated: current_block_height, + cause: cause.clone() + } + + } else { + application.stage + }; + + // New application computed + let new_application = hiring::Application { + active_role_staking_id: new_active_role_staking_id, + active_application_staking_id: new_active_application_staking_id, + stage: new_stage, + ..application + }; + + // Update to new application + >::insert(&application_id, new_application); + + // If the application is now finished compeleting any pending unstaking process, + // then we need to update the opening counters, and make the deactivation callback. + if is_now_done_unstaking { + // Update Opening + // We know the stage MUST be active, hence mutate is certain. + >::mutate(application.opening_id, |opening| { + // NB: This ugly byref destructuring is same issue as pointed out multiple times now. + if let hiring::OpeningStage::Active { + ref stage, + ref applicants, + active_application_count, + unstaking_application_count, + deactivated_application_count, + } = opening.stage + { + opening.stage = hiring::OpeningStage::Active { + stage: stage.clone(), + applicants: applicants.clone(), + active_application_count: active_application_count, + unstaking_application_count: unstaking_application_count - 1, + deactivated_application_count: deactivated_application_count + 1, + }; + } else { + assert!(false); + } + }); + + // Call handler + T::ApplicationDeactivatedHandler::deactivated(&application_id, &cause); + } + } +} + +/* + * ======== ======== ======== ======== ======= + * ======== PRIVATE TYPES AND METHODS ======== + * ======== ======== ======== ======== ======= + */ + +/* + * Used by multiple methods. + */ + +struct ApplicationsDeactivationsInitiationResult { + number_of_unstaking_applications: u32, + number_of_deactivated_applications: u32, +} +/* +impl ApplicationsDeactivationsInitiationResult { + + fn total(&self) -> u32 { + self.number_of_unstaking_applications + self.number_of_deactivated_applications + } +} +*/ +#[derive(PartialEq, Debug)] +enum ApplicationDeactivationInitationResult { + Ignored, // <= is there a case for kicking this out, making sure that initation cannot happen when it may fail? + Unstaking, + Deactivated, +} + +impl Module { + fn application_id_iter_to_map<'a>( + application_id_iter: impl Iterator, + ) -> BTreeMap> + { + application_id_iter + .map(|application_id| { + let application = >::get(application_id); + + (application_id.clone(), application) + }) + .collect::>() + } +} + +impl Module { + fn initiate_application_deactivations( + applications: &BTreeMap< + T::ApplicationId, + hiring::Application, + >, + application_stake_unstaking_period: Option, + role_stake_unstaking_period: Option, + cause: &ApplicationDeactivationCause, + ) -> ApplicationsDeactivationsInitiationResult { + // Update stage on active applications, and collect result + + applications + .iter() + .map( + |(application_id, application)| -> ApplicationDeactivationInitationResult { + // Initiate deactivations! + Self::try_to_initiate_application_deactivation( + application, + application_id.clone(), + application_stake_unstaking_period, + role_stake_unstaking_period, + cause, + ) + }, + ) + .fold( + // Initiatial reducer value + ApplicationsDeactivationsInitiationResult { + number_of_unstaking_applications: 0, + number_of_deactivated_applications: 0, + }, + |acc, deactivation_result| { + // Update accumulator counters based on what actually happened + match deactivation_result { + ApplicationDeactivationInitationResult::Ignored => acc, + + ApplicationDeactivationInitationResult::Unstaking => { + ApplicationsDeactivationsInitiationResult { + number_of_unstaking_applications: 1 + acc + .number_of_unstaking_applications, + number_of_deactivated_applications: acc + .number_of_deactivated_applications, + } + } + + ApplicationDeactivationInitationResult::Deactivated => { + ApplicationsDeactivationsInitiationResult { + number_of_unstaking_applications: acc + .number_of_unstaking_applications, + number_of_deactivated_applications: 1 + acc + .number_of_deactivated_applications, + } + } + } + }, + ) + } + + /// Initiates + fn try_to_initiate_application_deactivation( + application: &Application, + application_id: T::ApplicationId, + application_stake_unstaking_period: Option, + role_stake_unstaking_period: Option, + cause: &hiring::ApplicationDeactivationCause, + ) -> ApplicationDeactivationInitationResult { + match application.stage { + ApplicationStage::Active => { + // Initiate unstaking of any active stake for the application or the role. + let was_unstaked = Self::opt_infallible_unstake( + application.active_role_staking_id, + role_stake_unstaking_period, + ) || Self::opt_infallible_unstake( + application.active_application_staking_id, + application_stake_unstaking_period, + ); + + // Grab current block height + let current_block_height = >::block_number(); + + /* + * TODO: + * There should be a single transformation based on + * was_unstaked which renders a new value for `application.stage` + * and `opening.stage`, which guarantees to only produces new values + * for given variant values, but the state machine types are currently + * not well organised to support this. + * + * Likewise the construction of hiring::OpeningStage::Active below + * is a wreck because of this. + * + * Issue: https://github.com/Joystream/joystream/issues/36#issuecomment-539567407 + */ + + // Figure out new stage for the applcation + let new_application_stage = match was_unstaked { + true => ApplicationStage::Unstaking { + deactivation_initiated: current_block_height, + cause: cause.clone(), + }, + false => ApplicationStage::Inactive { + deactivation_initiated: current_block_height, + deactivated: current_block_height, + cause: cause.clone(), + }, + }; + + // Update the application stage + >::mutate(application_id, |application| { + application.stage = new_application_stage; + }); + + // Update counters on opening + >::mutate(application.opening_id, |opening| { + // NB: This ugly byref destructuring is same issue as pointed out multiple times now. + if let hiring::OpeningStage::Active { + ref stage, + ref applicants, + ref active_application_count, + ref unstaking_application_count, + ref deactivated_application_count, + } = opening.stage + { + let new_active_application_count = active_application_count - 1; + + let new_unstaking_application_count = + unstaking_application_count + if was_unstaked { 1 } else { 0 }; + + let new_deactivated_application_count = + deactivated_application_count + if was_unstaked { 0 } else { 1 }; + + opening.stage = hiring::OpeningStage::Active { + stage: (*stage).clone(), // <= truly horrible + applicants: (*applicants).clone(), // <= truly horrible + active_application_count: new_active_application_count, + unstaking_application_count: new_unstaking_application_count, + deactivated_application_count: new_deactivated_application_count, + }; + } else { + assert!(false); + } + }); + + // Call handler(s) + if was_unstaked { + T::ApplicationDeactivatedHandler::deactivated(&application_id, cause); + } + + // Return conclusion + match was_unstaked { + true => ApplicationDeactivationInitationResult::Unstaking, + false => ApplicationDeactivationInitationResult::Deactivated, + } + } + _ => ApplicationDeactivationInitationResult::Ignored, + } + } + + /// Tries to unstake, based on a stake id which, if set, MUST + /// be ready to be unstaked, with an optional unstaking period. + /// + /// + /// Returns whether unstaking was actually initiated. + fn opt_infallible_unstake( + opt_stake_id: Option, + opt_unstaking_period: Option, + ) -> bool { + if let Some(stake_id) = opt_stake_id { + // `initiate_unstaking` MUST hold, is runtime invariant, false means code is broken. + // But should we do panic in runtime? Is there safer way? + + assert!(>::initiate_unstaking( + &stake_id, + opt_unstaking_period.clone() + ) + .is_ok()); + } + + opt_stake_id.is_some() + } + + /// TODO: these also look messy + + fn opt_staking_policy_to_crowded_out_unstaking_period( + opt_staking_policy: &Option, T::BlockNumber>>, + ) -> Option { + if let Some(ref staking_policy) = opt_staking_policy { + staking_policy.crowded_out_unstaking_period_length + } else { + None + } + } +} + +impl Module { + fn infallible_opt_stake_initiation( + opt_imbalance: Option>, + application_id: &T::ApplicationId, + ) -> Option { + if let Some(imbalance) = opt_imbalance { + Some(Self::infallible_stake_initiation_on_application( + imbalance, + application_id, + )) + } else { + None + } + } + + fn infallible_stake_initiation_on_application( + imbalance: NegativeImbalance, + application_id: &T::ApplicationId, + ) -> T::StakeId { + // Create stake + let new_stake_id = >::create_stake(); + + // Keep track of this stake id to process unstaking callbacks that may + // be invoked later. + // NB: We purposefully update state to reflect mapping _before_ initiating staking below + // in order to be safe from race conditions arising out of third party code executing in callback of staking module. + + // MUST never already be a key for new stake, false means code is broken. + // But should we do panic in runtime? Is there safer way? + assert!(!>::exists(new_stake_id)); + + >::insert(new_stake_id, application_id); + + // Initiate staking + // + // MUST work, is runtime invariant, false means code is broken. + // But should we do panic in runtime? Is there safer way? + assert_eq!(>::stake(&new_stake_id, imbalance), Ok(())); + + new_stake_id + } +} + +/* + * Used by `add_application` method. + */ + +enum ApplicationAddedSuccess { + Unconditionally, + CrowdsOutExistingApplication(T::ApplicationId), +} + +enum ApplicationWouldGetAddedEvaluation { + No, + Yes(ApplicationAddedSuccess), +} + +impl Module { + /// Evaluates prospects for a new application + /// + fn would_application_get_added( + possible_opening_application_rationing_policy: &Option, + opening_applicants: &BTreeSet, + opt_role_stake_balance: &Option>, + opt_application_stake_balance: &Option>, + ) -> ApplicationWouldGetAddedEvaluation { + // Check whether any rationing policy is set at all, if not + // then there is no rationing, and any application can get added. + let application_rationing_policy = if let Some(application_rationing_policy) = + possible_opening_application_rationing_policy + { + application_rationing_policy + } else { + return ApplicationWouldGetAddedEvaluation::Yes( + ApplicationAddedSuccess::Unconditionally, + ); + }; + + // Map with applications + let applications_map = Self::application_id_iter_to_map(opening_applicants.iter()); + + // + let active_applications_with_stake_iter = + applications_map + .iter() + .filter_map(|(application_id, application)| { + if application.stage == hiring::ApplicationStage::Active { + let total_stake = + Self::get_opt_stake_amount(application.active_role_staking_id) + + Self::get_opt_stake_amount( + application.active_application_staking_id, + ); + + Some((application_id, application, total_stake)) + } else { + None + } + }); + + // Compute number of active applications + let number_of_active_applications = active_applications_with_stake_iter.clone().count(); + + // Check whether the current number of _active_ applicants is either at or above the maximum + // limit, if not, then we can add at least one additional application, + // otherwise we must evaluate whether this new application would specifically get added. + if (number_of_active_applications as u32) + < application_rationing_policy.max_active_applicants + { + return ApplicationWouldGetAddedEvaluation::Yes( + ApplicationAddedSuccess::Unconditionally, + ); + } + + // Here we try to figure out if the new application + // has sufficient stake to crowd out one of the already + // active applicants. + + // The total stake of new application + let total_stake_of_new_application = opt_role_stake_balance.unwrap_or_default() + + opt_application_stake_balance.unwrap_or_default(); + + // The total stake of all current active applications + let opt_min_item = active_applications_with_stake_iter + .clone() + .min_by_key(|(_, _, total_stake)| total_stake.clone()); + + // MUST hold , since guard above guarantees that `number_of_active_applications` + // which is length of `active_applications_iter`, > 0. + assert!(opt_min_item.is_some()); + + let (application_id, _, lowest_active_total_stake) = opt_min_item.unwrap(); + + // Finally we compare the two and come up with a final evaluation + if total_stake_of_new_application <= lowest_active_total_stake { + ApplicationWouldGetAddedEvaluation::No // stake too low! + } else { + ApplicationWouldGetAddedEvaluation::Yes( + ApplicationAddedSuccess::CrowdsOutExistingApplication(application_id.clone()), + ) + } + } + + fn get_opt_stake_amount(stake_id: Option) -> BalanceOf { + stake_id.map_or( as Zero>::zero(), |stake_id| { + // INVARIANT: stake MUST exist in the staking module + assert!(>::exists(stake_id)); + + let stake = >::get(stake_id); + + match stake.staking_status { + // INVARIANT: stake MUST be in the staked state. + stake::StakingStatus::Staked(staked_state) => staked_state.staked_amount, + _ => { + assert!(false); + Zero::zero() + } + } + }) + } +} + +/* + * Used by unstaked method. + */ + +// ... + +/* + * Used by `on_initialized` + */ + +impl Module { + fn opt_staking_policy_to_review_period_expired_unstaking_period( + opt_staking_policy: &Option, T::BlockNumber>>, + ) -> Option { + if let Some(ref staking_policy) = opt_staking_policy { + staking_policy.review_period_expired_unstaking_period_length + } else { + None + } + } +} diff --git a/src/macroes.rs b/src/macroes.rs new file mode 100644 index 0000000000..266a25ce01 --- /dev/null +++ b/src/macroes.rs @@ -0,0 +1,319 @@ +/// TODO: Move into Substrate-utility library +/// Ensure that two expressions are equal. +/// +/// +/// Returns ... +#[macro_export] +macro_rules! ensure_eq { + ($left:expr, $right:expr, $error:expr) => { + ensure!(($left) == ($right), $error) + }; +} + +/// TODO: Move into Substrate-utility library +/// Ensure that a storage map, with a given name, has mapping for the given key value. +/// +/// Returns ... +#[macro_export] +macro_rules! ensure_map_has_mapping_with_key { + ($map_variable_name:ident , $runtime_trait:tt, $key:expr, $error:expr) => {{ + if <$map_variable_name<$runtime_trait>>::exists($key) { + let value = <$map_variable_name<$runtime_trait>>::get($key); + + Ok(value) + } else { + Err($error) + } + }}; +} + +/// Ensure that an opening exists in `OpeningnById`, and if so, return it. +/// +/// Returns +/// - `Ok(opening)` where `opening` is the opening, if it exists. +/// - `Err($error)` otherwise +#[macro_export] +macro_rules! ensure_opening_exists { + ($runtime_trait:tt, $opening_id:expr, $error:expr) => {{ + ensure_map_has_mapping_with_key!(OpeningById, $runtime_trait, $opening_id, $error) + }}; +} + +/* + * Move this out later + * + + ($runtime_trait:tt, $application_id:expr, $error:expr, auto_fetch_opening) => {{ + + let application = ensure_map_has_mapping_with_key!( + ApplicationById, + $runtime_trait, + $application_id, + $error + )?; + + // Grab corresponding opening, which MUST exist. + let opening = >::get(application.opening_id); + + // Return both + Ok((application, opening)) + }}; +*/ + +/// Ensure that an applications exists in `ApplicationById` , and if so, return it along with the +/// corresponding opening. +/// +/// Returns... +#[macro_export] +macro_rules! ensure_application_exists { + ($runtime_trait:tt, $application_id:expr, $error:expr) => {{ + ensure_map_has_mapping_with_key!(ApplicationById, $runtime_trait, $application_id, $error) + }}; + + ($runtime_trait:tt, $application_id:expr, $error:expr, auto_fetch_opening) => {{ + ensure_application_exists!($runtime_trait, $application_id, $error).and_then( + |application| { + // Grab corresponding opening, which MUST exist. + let opening = >::get(application.opening_id); + + // Return both + Ok((application, opening)) + }, + ) + }}; +} + +/// Ensures that an opening is active. +/// +/// Returns ... +#[macro_export] +macro_rules! ensure_opening_is_active { + ($stage:expr, $error:expr) => {{ + match $stage { + hiring::OpeningStage::Active { + // <= need proper type here in the future, not param + stage, + applicants, + active_application_count, + unstaking_application_count, + deactivated_application_count, + } => Ok(( + stage, + applicants, + active_application_count, + unstaking_application_count, + deactivated_application_count, + )), + _ => Err($error), + } + }}; +} + +/// Ensures that an opening is waiting to begin. +/// +/// Returns ... +#[macro_export] +macro_rules! ensure_opening_stage_is_waiting_to_begin { + ($stage:expr, $error:expr) => {{ + match $stage { + hiring::OpeningStage::WaitingToBegin { + // <= need proper type here in the future, not param + begins_at_block, + } => Ok(begins_at_block), + _ => Err($error), + } + }}; +} + +/* +//// FAILED ATTEMPT AT GENERLIZATION +/// Ensures that .... +/// +/// Returns ... +#[macro_export] +macro_rules! ensure_is_variant { + + ($stage:expr, $variant_path:path, $error:expr, $( $x:expr ),*) => {{ + + match $stage { + + $variant_path { $($x,)* } => + Ok(( $($x,)* )) + , + _ => Err($error), + } + }} + + +} +*/ + +/// Ensures that active opening stage is accepting applications. +/// +/// Returns ... +#[macro_export] +macro_rules! ensure_active_opening_is_accepting_applications { + + ($stage:expr, $error:expr) => {{ + + match $stage { + hiring::ActiveOpeningStage::AcceptingApplications { + started_accepting_applicants_at_block + } => Ok(started_accepting_applicants_at_block), // <= need proper type here in the future, not param + _ => Err($error), + } + }} +} + +/// Ensures that active opening stage is in review period. +/// +/// Returns ... +#[macro_export] +macro_rules! ensure_active_opening_is_in_review_period { + ($stage:expr, $error:expr) => {{ + match $stage { + hiring::ActiveOpeningStage::ReviewPeriod { + started_accepting_applicants_at_block, + started_review_period_at_block, + } => Ok(( + started_accepting_applicants_at_block, + started_review_period_at_block, + )), // <= need proper type here in the future, not param + _ => Err($error), + } + }}; +} + +/// Ensures that optional imbalance matches requirements of optional staking policy +/// +/// Returns ... +#[macro_export] +macro_rules! ensure_stake_imbalance_matches_staking_policy { + ( + $opt_imbalance:expr, + $opt_policy: expr, + $stake_missing_when_required_error:expr, + $stake_provided_when_redundant_error:expr, + $stake_amount_too_low_error:expr + + ) => {{ + if let Some(ref imbalance) = $opt_imbalance { + if let Some(ref policy) = $opt_policy { + let imbalance_value = imbalance.peek(); + + if !policy.accepts_amount(&imbalance_value) { + Err($stake_amount_too_low_error) + } else { + Ok(Some(imbalance_value)) + } + } else { + Err($stake_provided_when_redundant_error) + } + } else if $opt_policy.is_some() { + Err($stake_missing_when_required_error) + } else { + Ok(None) + } + }}; +} + +/// Ensures that an optional unstaking period is at least one block whens set. +/// +/// Returns ... +#[macro_export] +macro_rules! _ensure_opt_unstaking_period_not_zero { + ($opt_period:expr, $error:expr) => {{ + if let Some(ref length) = $opt_period { + let lower_bound = One::one(); + + if *length < lower_bound { + Err($error) + } else { + Ok(()) + } + } else { + Ok(()) + } + }}; +} + +/// Ensures that ... +/// +/// Returns ... +#[macro_export] +macro_rules! _ensure_opt_unstaking_period_not_redundant { + ( + $opt_policy:expr, + $opt_staking_period:expr, + $error:expr + ) => {{ + if $opt_policy.is_some() || $opt_staking_period.is_none() { + Ok(()) + } else { + Err($error) + } + }}; +} + +/// Ensures that ... +/// +/// Returns ... +#[macro_export] +macro_rules! ensure_opt_unstaking_period_is_ok { + ( + $opt_staking_period:expr, + $opt_staking_policy:expr, + $period_zero_error:expr, + $period_redundant_error:expr + ) => {{ + _ensure_opt_unstaking_period_not_zero!($opt_staking_period, $period_zero_error).and( + _ensure_opt_unstaking_period_not_redundant!( + $opt_staking_policy, + $opt_staking_period, + $period_redundant_error + ), + ) + }}; +} + +/// Ensures that optional staking policy prescribes value that clears minimum balance requirement of the `Currency` module. +/// +/// Returns ... +#[macro_export] +macro_rules! ensure_amount_valid_in_opt_staking_policy { + ($runtime_trait:tt, $opt_staking_policy:expr, $error:expr) => {{ + if let Some(ref staking_policy) = $opt_staking_policy { + if staking_policy.amount < $runtime_trait::Currency::minimum_balance() { + Err($error) + } else { + Ok(()) + } + } else { + Ok(()) + } + }}; +} + +/// Ensures that a new application would make it into a given opening +/// +/// Returns ... +#[macro_export] +macro_rules! ensure_application_would_get_added { + ( + $opt_staking_policy:expr, + $applicants:expr, + $opt_role_stake_balance:expr, + $opt_application_stake_balance:expr, + $error:expr) => {{ + match Self::would_application_get_added( + $opt_staking_policy, + $applicants, + $opt_role_stake_balance, + $opt_application_stake_balance, + ) { + // Would get added indeed! + ApplicationWouldGetAddedEvaluation::Yes(success) => Ok(success), + _ => Err($error), + } + }}; +} From 3a80b421d558c488951aa9a4ffa046701f6dbe69 Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Fri, 18 Oct 2019 12:33:35 +0200 Subject: [PATCH 04/25] Mocking --- src/mock.rs | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 src/mock.rs diff --git a/src/mock.rs b/src/mock.rs new file mode 100644 index 0000000000..c0e9972b5d --- /dev/null +++ b/src/mock.rs @@ -0,0 +1,113 @@ +#![cfg(test)] + +use crate::*; + +use primitives::{Blake2Hasher, H256}; + +use crate::{Module, Trait}; +use balances; +use runtime_primitives::{ + testing::Header, + traits::{BlakeTwo256, IdentityLookup}, + Perbill, +}; +use srml_support::{impl_outer_origin, parameter_types}; + +use stake; + +impl_outer_origin! { + pub enum Origin for Test {} +} + +// Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. +#[derive(Clone, PartialEq, Eq, Debug)] +pub struct Test; +parameter_types! { + pub const BlockHashCount: u64 = 250; + pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; + pub const AvailableBlockRatio: Perbill = Perbill::one(); + pub const MinimumPeriod: u64 = 5; +} + +impl system::Trait for Test { + type Origin = Origin; + type Index = u64; + type BlockNumber = u64; + type Call = (); + type Hash = H256; + type Hashing = BlakeTwo256; + type AccountId = u64; + type Lookup = IdentityLookup; + type Header = Header; + type WeightMultiplierUpdate = (); + type Event = (); + type BlockHashCount = BlockHashCount; + type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockLength = MaximumBlockLength; + type AvailableBlockRatio = AvailableBlockRatio; + type Version = (); +} + +parameter_types! { + pub const ExistentialDeposit: u32 = 0; + pub const TransferFee: u32 = 0; + pub const CreationFee: u32 = 0; + pub const TransactionBaseFee: u32 = 1; + pub const TransactionByteFee: u32 = 0; + pub const InitialMembersBalance: u64 = 2000; + pub const StakePoolId: [u8; 8] = *b"joystake"; +} + +impl balances::Trait for Test { + /// The type for recording an account's balance. + type Balance = u64; + /// What to do if an account's free balance gets zeroed. + type OnFreeBalanceZero = (); + /// What to do if a new account is created. + type OnNewAccount = (); + /// The ubiquitous event type. + type Event = (); + + type TransactionPayment = (); + type DustRemoval = (); + type TransferPayment = (); + type ExistentialDeposit = ExistentialDeposit; + type TransferFee = TransferFee; + type CreationFee = CreationFee; + type TransactionBaseFee = TransactionBaseFee; + type TransactionByteFee = TransactionByteFee; + type WeightToFee = (); +} + +impl Trait for Test { + //type Event = (); + + type OpeningId = u64; + + type ApplicationId = u64; + + //type Currency = Balances; + + type ApplicationDeactivatedHandler = (); +} + +impl stake::Trait for Test { + type Currency = Balances; + type StakePoolId = StakePoolId; + type StakingEventsHandler = (); + type StakeId = u64; + type SlashId = u64; +} + +pub fn build_test_externalities() -> runtime_io::TestExternalities { + let t = system::GenesisConfig::default() + .build_storage::() + .unwrap(); + + t.into() +} + +//pub type System = system::Module; +pub type Balances = balances::Module; +pub type Hiring = Module; From 0e12cee6e992ff664020be08c6264d0cde8227f8 Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Fri, 18 Oct 2019 12:33:46 +0200 Subject: [PATCH 05/25] Initial work on tests --- src/test.rs | 184 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 184 insertions(+) create mode 100644 src/test.rs diff --git a/src/test.rs b/src/test.rs new file mode 100644 index 0000000000..279b6de721 --- /dev/null +++ b/src/test.rs @@ -0,0 +1,184 @@ +#![cfg(test)] + +use super::*; +use crate::mock::*; + +use runtime_io::with_externalities; + +static FIRST_BLOCK_HEIGHT: ::BlockNumber = 1; + +use rstd::collections::btree_set::BTreeSet; + +/// add_opening + +#[test] +fn add_opening_success_waiting_to_begin() { + with_externalities(&mut build_test_externalities(), || { + // FIXTURES + + let expected_opening_id = 0; + + let activation_at = ActivateOpeningAt::CurrentBlock; + let max_review_period_length = 672; + let application_rationing_policy = None; + let application_staking_policy = None; + let role_staking_policy = None; + let human_readable_text = "blablabalbal".as_bytes().to_vec(); + + // Add an opening, check that the returned value is Zero + assert_eq!( + Hiring::add_opening( + activation_at, + max_review_period_length, + application_rationing_policy, + application_staking_policy, + role_staking_policy, + human_readable_text.clone() + ), + Ok(expected_opening_id) + ); + + // Check that next opening id has been updated + assert_eq!(Hiring::next_opening_id(), expected_opening_id + 1); + + // Check that our opening actually was added + assert!(>::exists(expected_opening_id)); + + let found_opening = Hiring::opening_by_id(expected_opening_id); + + assert_eq!( + found_opening, + Opening { + created: FIRST_BLOCK_HEIGHT, + stage: OpeningStage::Active { + stage: ActiveOpeningStage::AcceptingApplications { + started_accepting_applicants_at_block: FIRST_BLOCK_HEIGHT + }, + applicants: BTreeSet::new(), + active_application_count: 0, + unstaking_application_count: 0, + deactivated_application_count: 0 + }, + max_review_period_length: max_review_period_length, + application_rationing_policy: None, //application_rationing_policy, + application_staking_policy: None, //application_staking_policy, + role_staking_policy: None, //role_staking_policy, + human_readable_text: human_readable_text.clone() + } + ); + }); +} + +#[test] +fn add_opening_fails_due_to_activation_in_the_past() { + with_externalities(&mut build_test_externalities(), || { + // FIXTURES + + let expected_opening_id = 0; + + let activation_at = ActivateOpeningAt::ExactBlock(0); + let max_review_period_length = 672; + let application_rationing_policy = None; + let application_staking_policy = None; + let role_staking_policy = None; + let human_readable_text = "blablabalbal".as_bytes().to_vec(); + + // Add an opening, check that the returned value is Zero + assert_eq!( + Hiring::add_opening( + activation_at, + max_review_period_length, + application_rationing_policy, + application_staking_policy, + role_staking_policy, + human_readable_text.clone() + ), + Err(AddOpeningError::OpeningMustActivateInTheFuture) + ); + + // Check that next opening id has been updated + assert_eq!(Hiring::next_opening_id(), expected_opening_id); + + // Check that our opening actually was not added + assert!(!>::exists(expected_opening_id)); + }); +} + +/// cancel_opening + +#[test] +fn cancel_opening_success() { + with_externalities(&mut build_test_externalities(), || {}); +} + +#[test] +fn cancel_opening_fails_due_to_too_short_application_unstaking_period() { + with_externalities(&mut build_test_externalities(), || {}); +} + +#[test] +fn cancel_opening_fails_due_to_too_short_role_unstaking_period() { + with_externalities(&mut build_test_externalities(), || {}); +} + +#[test] +fn cancel_opening_fails_due_to_opening_not_existing() { + with_externalities(&mut build_test_externalities(), || {}); +} + +// + +/** + * remove_opening + * + * + */ + +/** + * begin_accepting_applications + * + * begin_accepting_applications_succeeded + * begin_accepting_applications_fails_due_to_invalid_opening_id + * begin_accepting_applications_fails_due_to_opening_not_being_in_waiting_to_begin_stage + */ + +/** + * begin_review + * + * begin_review + */ + +/** + * fill_opening + * + * fill_opening + */ + +/** + * add_application + * + * add_application + */ + +/** + * deactive_application + * + * deactive_application + */ + +/** + * remove_application + * + * remove_application + */ + +/** + * unstaked + * + * unstaked + */ + +#[test] +fn foo() { + with_externalities(&mut build_test_externalities(), || {}); +} From 45d2deb12bc0dc11f58f60e005fb4601ce76282e Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Thu, 31 Oct 2019 12:48:37 +0100 Subject: [PATCH 06/25] Replace standard iterator with Substrate iterator Not available when building WASM target --- src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 6daa6c9ef0..4f4fe81291 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,7 +11,8 @@ use srml_support::traits::Currency; use srml_support::{ decl_module, decl_storage, ensure, EnumerableStorageMap, Parameter, StorageMap, StorageValue, }; -use std::iter::Iterator; + +use rstd::iter::Iterator; use runtime_primitives::traits::Zero; From fc04a0a311301920b4a17bd0ae68d3cb26b5b1a0 Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Thu, 31 Oct 2019 12:56:48 +0100 Subject: [PATCH 07/25] Drop dangling attribute --- src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 4f4fe81291..a859300338 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,8 +1,6 @@ // Ensure we're `no_std` when compiling for Wasm. #![cfg_attr(not(feature = "std"), no_std)] -#[cfg(feature = "std")] -//use serde_derive::{Deserialize, Serialize}; use rstd::prelude::*; use codec::{Codec, Decode, Encode}; From 2beab3af0d0abe71f11cffd53cb30961f05d850c Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Thu, 31 Oct 2019 13:23:19 +0100 Subject: [PATCH 08/25] Missing inclusion of Substrate standard lib Normal build hides this, since normal Rust standard library is added implicitly. --- src/hiring.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hiring.rs b/src/hiring.rs index df3af830f9..a9661adccf 100644 --- a/src/hiring.rs +++ b/src/hiring.rs @@ -4,7 +4,7 @@ use codec::{Decode, Encode}; //use srml_support::ensure; //use rstd::collections::btree_map::BTreeMap; use rstd::collections::btree_set::BTreeSet; - +use rstd::prelude::*; //use crate::{Trait}; // Possible causes From c030ae8305ef81e59934125c0927c92c4cb77ef9 Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Thu, 31 Oct 2019 16:16:03 +0100 Subject: [PATCH 09/25] Expose pub types at crate level --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index a859300338..51f2f7576e 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,7 +24,7 @@ mod macroes; mod mock; mod test; -use hiring::*; +pub use hiring::*; use system; use stake; From c0a8729a3808a14e4ea2799a6fb2399584af84b3 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Fri, 1 Nov 2019 17:02:37 +0200 Subject: [PATCH 10/25] update dependencies substrate and staking module --- Cargo.toml | 22 +++++++++++----------- src/lib.rs | 10 ++++------ src/mock.rs | 9 ++------- src/test.rs | 16 +++++++--------- 4 files changed, 24 insertions(+), 33 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a0cd6d82b0..e5a9c6192d 100755 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,35 +8,35 @@ edition = '2018' hex-literal = '0.1.0' serde = { version = '1.0', optional = true } serde_derive = { version = '1.0', optional = true } -rstd = { package = 'sr-std', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} -runtime-primitives = { package = 'sr-primitives', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} -srml-support = { package = 'srml-support', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} -srml-support-procedural = { package = 'srml-support-procedural', git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} -system = { package = 'srml-system', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} -balances = { package = 'srml-balances', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} +rstd = { package = 'sr-std', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = '0e3001a1ad6fa3d1ba7da7342a8d0d3b3facb2f3'} +runtime-primitives = { package = 'sr-primitives', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = '0e3001a1ad6fa3d1ba7da7342a8d0d3b3facb2f3'} +srml-support = { package = 'srml-support', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = '0e3001a1ad6fa3d1ba7da7342a8d0d3b3facb2f3'} +srml-support-procedural = { package = 'srml-support-procedural', git = 'https://github.com/paritytech/substrate.git', rev = '0e3001a1ad6fa3d1ba7da7342a8d0d3b3facb2f3'} +system = { package = 'srml-system', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = '0e3001a1ad6fa3d1ba7da7342a8d0d3b3facb2f3'} +balances = { package = 'srml-balances', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = '0e3001a1ad6fa3d1ba7da7342a8d0d3b3facb2f3'} codec = { package = 'parity-scale-codec', version = '1.0.0', default-features = false, features = ['derive'] } [dependencies.stake] default_features = false git = 'https://github.com/mnaamani/substrate-stake' package = 'substrate-stake-module' -rev = 'eb4ef9711f1c375cc59f55b74b8a4e725335f4f0' # branch = 'staking' +rev = '28f4815a7d020fdcd450095cc8a8d7076f8899a6' # branch = 'staking' [dependencies.timestamp] default_features = false git = 'https://github.com/paritytech/substrate.git' package = 'srml-timestamp' -rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b' +rev = '0e3001a1ad6fa3d1ba7da7342a8d0d3b3facb2f3' [dependencies.runtime-io] default_features = false git = 'https://github.com/paritytech/substrate.git' package = 'sr-io' -rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b' +rev = '0e3001a1ad6fa3d1ba7da7342a8d0d3b3facb2f3' [dev-dependencies] -runtime-io = { package = 'sr-io', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} -primitives = { package = 'substrate-primitives', git = 'https://github.com/paritytech/substrate.git', rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b'} +runtime-io = { package = 'sr-io', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = '0e3001a1ad6fa3d1ba7da7342a8d0d3b3facb2f3'} +primitives = { package = 'substrate-primitives', git = 'https://github.com/paritytech/substrate.git', rev = '0e3001a1ad6fa3d1ba7da7342a8d0d3b3facb2f3'} [features] default = ['std'] diff --git a/src/lib.rs b/src/lib.rs index 51f2f7576e..e65e1cd676 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,11 +4,9 @@ use rstd::prelude::*; use codec::{Codec, Decode, Encode}; -use runtime_primitives::traits::{MaybeSerializeDebug, Member, One, SimpleArithmetic}; +use runtime_primitives::traits::{MaybeSerialize, Member, One, SimpleArithmetic}; use srml_support::traits::Currency; -use srml_support::{ - decl_module, decl_storage, ensure, EnumerableStorageMap, Parameter, StorageMap, StorageValue, -}; +use srml_support::{decl_module, decl_storage, ensure, Parameter}; use rstd::iter::Iterator; @@ -43,7 +41,7 @@ pub trait Trait: system::Trait + stake::Trait + Sized { + Codec + Default + Copy - + MaybeSerializeDebug + + MaybeSerialize + PartialEq; type ApplicationId: Parameter @@ -52,7 +50,7 @@ pub trait Trait: system::Trait + stake::Trait + Sized { + Codec + Default + Copy - + MaybeSerializeDebug + + MaybeSerialize + PartialEq; /// Type that will handle various staking events diff --git a/src/mock.rs b/src/mock.rs index c0e9972b5d..e911bfbf9b 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -2,7 +2,7 @@ use crate::*; -use primitives::{Blake2Hasher, H256}; +use primitives::H256; use crate::{Module, Trait}; use balances; @@ -40,7 +40,6 @@ impl system::Trait for Test { type AccountId = u64; type Lookup = IdentityLookup; type Header = Header; - type WeightMultiplierUpdate = (); type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; @@ -69,15 +68,11 @@ impl balances::Trait for Test { /// The ubiquitous event type. type Event = (); - type TransactionPayment = (); type DustRemoval = (); type TransferPayment = (); type ExistentialDeposit = ExistentialDeposit; type TransferFee = TransferFee; type CreationFee = CreationFee; - type TransactionBaseFee = TransactionBaseFee; - type TransactionByteFee = TransactionByteFee; - type WeightToFee = (); } impl Trait for Test { @@ -100,7 +95,7 @@ impl stake::Trait for Test { type SlashId = u64; } -pub fn build_test_externalities() -> runtime_io::TestExternalities { +pub fn build_test_externalities() -> runtime_io::TestExternalities { let t = system::GenesisConfig::default() .build_storage::() .unwrap(); diff --git a/src/test.rs b/src/test.rs index 279b6de721..e6d731ba54 100644 --- a/src/test.rs +++ b/src/test.rs @@ -3,8 +3,6 @@ use super::*; use crate::mock::*; -use runtime_io::with_externalities; - static FIRST_BLOCK_HEIGHT: ::BlockNumber = 1; use rstd::collections::btree_set::BTreeSet; @@ -13,7 +11,7 @@ use rstd::collections::btree_set::BTreeSet; #[test] fn add_opening_success_waiting_to_begin() { - with_externalities(&mut build_test_externalities(), || { + build_test_externalities().execute_with(|| { // FIXTURES let expected_opening_id = 0; @@ -71,7 +69,7 @@ fn add_opening_success_waiting_to_begin() { #[test] fn add_opening_fails_due_to_activation_in_the_past() { - with_externalities(&mut build_test_externalities(), || { + build_test_externalities().execute_with(|| { // FIXTURES let expected_opening_id = 0; @@ -108,22 +106,22 @@ fn add_opening_fails_due_to_activation_in_the_past() { #[test] fn cancel_opening_success() { - with_externalities(&mut build_test_externalities(), || {}); + build_test_externalities().execute_with(|| {}); } #[test] fn cancel_opening_fails_due_to_too_short_application_unstaking_period() { - with_externalities(&mut build_test_externalities(), || {}); + build_test_externalities().execute_with(|| {}); } #[test] fn cancel_opening_fails_due_to_too_short_role_unstaking_period() { - with_externalities(&mut build_test_externalities(), || {}); + build_test_externalities().execute_with(|| {}); } #[test] fn cancel_opening_fails_due_to_opening_not_existing() { - with_externalities(&mut build_test_externalities(), || {}); + build_test_externalities().execute_with(|| {}); } // @@ -180,5 +178,5 @@ fn cancel_opening_fails_due_to_opening_not_existing() { #[test] fn foo() { - with_externalities(&mut build_test_externalities(), || {}); + build_test_externalities().execute_with(|| {}); } From 63e0cffd0b2f6872b252aab674b09e3cd0235872 Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Mon, 11 Nov 2019 10:52:29 +0100 Subject: [PATCH 11/25] Removal of redundant generic trait constraint --- src/hiring.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hiring.rs b/src/hiring.rs index a9661adccf..7e4a0f9eb8 100644 --- a/src/hiring.rs +++ b/src/hiring.rs @@ -261,7 +261,7 @@ impl StakingPolicy { +pub struct Opening { // Block at which opening was added pub created: BlockNumber, From 8336c45caa94d7b991a4396925c37435e99a9984 Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Mon, 11 Nov 2019 13:53:47 +0100 Subject: [PATCH 12/25] Need cloning for for downstream working group user --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index e65e1cd676..a3beb5f699 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -276,7 +276,7 @@ pub enum StakePurpose { } // Safe and explict way of chosing -#[derive(Encode, Decode, Debug, Eq, PartialEq)] +#[derive(Encode, Decode, Debug, Eq, PartialEq, Clone)] pub enum ActivateOpeningAt { CurrentBlock, ExactBlock(T::BlockNumber), From 7fb10423c8d2f175692daef027dea661b2fe8256 Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Mon, 11 Nov 2019 15:06:34 +0100 Subject: [PATCH 13/25] Default impl for enums doesn't work --- src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index a3beb5f699..01b16420cc 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -276,12 +276,19 @@ pub enum StakePurpose { } // Safe and explict way of chosing -#[derive(Encode, Decode, Debug, Eq, PartialEq, Clone)] +#[derive(Encode, Decode, Eq, PartialEq, Clone)] pub enum ActivateOpeningAt { CurrentBlock, ExactBlock(T::BlockNumber), } +impl Default for ActivateOpeningAt { + + fn default() -> Self { + ActivateOpeningAt::CurrentBlock + } +} + #[derive(Debug, PartialEq)] pub enum AddOpeningError { OpeningMustActivateInTheFuture, From c3bbc67be1ad1891874abcc8ec9ec0e612c475cd Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Mon, 11 Nov 2019 15:27:59 +0100 Subject: [PATCH 14/25] Broken Debug trait constraint downstream was core issue, not Default --- src/lib.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 01b16420cc..d94e565c9f 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -276,17 +276,10 @@ pub enum StakePurpose { } // Safe and explict way of chosing -#[derive(Encode, Decode, Eq, PartialEq, Clone)] -pub enum ActivateOpeningAt { +#[derive(Encode, Decode, Eq, PartialEq, Clone, Debug)] +pub enum ActivateOpeningAt { CurrentBlock, - ExactBlock(T::BlockNumber), -} - -impl Default for ActivateOpeningAt { - - fn default() -> Self { - ActivateOpeningAt::CurrentBlock - } + ExactBlock(BlockNumber), } #[derive(Debug, PartialEq)] @@ -374,7 +367,7 @@ pub enum DeactivateApplicationError { impl Module { pub fn add_opening( - activate_at: ActivateOpeningAt, + activate_at: ActivateOpeningAt, max_review_period_length: T::BlockNumber, application_rationing_policy: Option, application_staking_policy: Option, T::BlockNumber>>, From d1a5491d5855115632d870cfb6118edaf48c6a7f Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Tue, 19 Nov 2019 17:38:46 +0100 Subject: [PATCH 15/25] Added ensure_can_add_application Is required to avoid caller burden of cleaning up imbalance parameters in failure outcome of add_application. --- src/lib.rs | 119 +++++++++++++++++++++++++++++++++++++------------ src/macroes.rs | 56 ++++++++++++++++++----- 2 files changed, 135 insertions(+), 40 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index d94e565c9f..1967064c8b 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -343,6 +343,24 @@ pub enum BeginAcceptingApplicationsError { OpeningIsNotInWaitingToBeginStage, } +pub struct DestructuredApplicationCanBeAddedEvaluation { + + pub opening: Opening, T::BlockNumber, T::ApplicationId>, + + pub active_stage: ActiveOpeningStage, + + pub applicants: BTreeSet, + + pub active_application_count: u32, + + pub unstaking_application_count: u32, + + pub deactivated_application_count: u32, + + pub would_get_added_success: ApplicationAddedSuccess + +} + pub enum AddApplicationError { OpeningDoesNotExist, StakeProvidedWhenRedundant(StakePurpose), @@ -824,29 +842,27 @@ impl Module { /// the role, the application or both possibly. /// /// Returns .. - pub fn add_application( + pub fn ensure_can_add_application( opening_id: T::OpeningId, - opt_role_stake_imbalance: Option>, - opt_application_stake_imbalance: Option>, - human_readable_text: Vec, - ) -> Result, AddApplicationError> { - // NB: Should we provide the two imbalances back?? + opt_role_stake_balance: Option>, + opt_application_stake_balance: Option> + ) -> Result, AddApplicationError> { // Ensure that the opening exists let opening = ensure_opening_exists!(T, opening_id, AddApplicationError::OpeningDoesNotExist)?; // Ensure that proposed stakes match the policy of the opening. - let opt_role_stake_balance = ensure_stake_imbalance_matches_staking_policy!( - &opt_role_stake_imbalance, + let opt_role_stake_balance = ensure_stake_balance_matches_staking_policy!( + &opt_role_stake_balance, &opening.role_staking_policy, AddApplicationError::StakeMissingWhenRequired(StakePurpose::Role), AddApplicationError::StakeProvidedWhenRedundant(StakePurpose::Role), AddApplicationError::StakeAmountTooLow(StakePurpose::Role) )?; - let opt_application_stake_balance = ensure_stake_imbalance_matches_staking_policy!( - &opt_application_stake_imbalance, + let opt_application_stake_balance = ensure_stake_balance_matches_staking_policy!( + &opt_application_stake_balance, &opening.application_staking_policy, AddApplicationError::StakeMissingWhenRequired(StakePurpose::Application), AddApplicationError::StakeProvidedWhenRedundant(StakePurpose::Application), @@ -872,7 +888,7 @@ impl Module { )?; // Ensure that the new application would actually make it - let success = ensure_application_would_get_added!( + let would_get_added_success = ensure_application_would_get_added!( &opening.application_rationing_policy, &applicants, &opt_role_stake_balance, @@ -880,6 +896,46 @@ impl Module { AddApplicationError::NewApplicationWasCrowdedOut )?; + + Ok(DestructuredApplicationCanBeAddedEvaluation{ + opening: opening, + active_stage: active_stage, + applicants: applicants, + active_application_count: active_application_count, + unstaking_application_count: unstaking_application_count, + deactivated_application_count: deactivated_application_count, + would_get_added_success: would_get_added_success + }) + } + + /// Adds a new application on the given opening, and begins staking for + /// the role, the application or both possibly. + /// + /// Returns .. + pub fn add_application( + opening_id: T::OpeningId, + opt_role_stake_imbalance: Option>, + opt_application_stake_imbalance: Option>, + human_readable_text: Vec, + ) -> Result, AddApplicationError> { + + let opt_role_stake_balance = + if let Some(ref imbalance) = opt_role_stake_imbalance { + Some(imbalance.peek()) + } else { + None + }; + + let opt_application_stake_balance = + if let Some(ref imbalance) = opt_application_stake_imbalance { + Some(imbalance.peek()) + } else { + None + }; + + + let can_be_added_destructured = Self::ensure_can_add_application(opening_id, opt_role_stake_balance, opt_application_stake_balance)?; + // // == MUTATION SAFE == // @@ -887,16 +943,16 @@ impl Module { // If required, deactive another application that was crowded out. if let ApplicationAddedSuccess::CrowdsOutExistingApplication( id_of_croweded_out_application, - ) = success + ) = can_be_added_destructured.would_get_added_success { // Get relevant unstaking periods let opt_application_stake_unstaking_period = Self::opt_staking_policy_to_crowded_out_unstaking_period( - &opening.application_staking_policy, + &can_be_added_destructured.opening.application_staking_policy, ); let opt_role_stake_unstaking_period = Self::opt_staking_policy_to_crowded_out_unstaking_period( - &opening.role_staking_policy, + &can_be_added_destructured.opening.role_staking_policy, ); // Fetch application @@ -938,7 +994,9 @@ impl Module { // Compute index for this new application // TODO: fix so that `number_of_appliations_ever_added` can be invoked. let application_index_in_opening = - active_application_count + unstaking_application_count + deactivated_application_count; // cant do this due to bad design of stage => opening.stage.number_of_appliations_ever_added(); + can_be_added_destructured.active_application_count + + can_be_added_destructured.unstaking_application_count + + can_be_added_destructured.deactivated_application_count; // cant do this due to bad design of stage => opening.stage.number_of_appliations_ever_added(); // Create a new application let new_application = hiring::Application { @@ -958,22 +1016,25 @@ impl Module { >::mutate(|id| *id += One::one()); // Update counter on opening + + /* + TODO: + Yet another instance of problems due to not following https://github.com/Joystream/joystream/issues/36#issuecomment-539567407 + */ + let new_active_stage = hiring::OpeningStage::Active { + stage: can_be_added_destructured.active_stage, + applicants: can_be_added_destructured.applicants, + active_application_count: can_be_added_destructured.active_application_count + 1, + unstaking_application_count: can_be_added_destructured.unstaking_application_count, + deactivated_application_count: can_be_added_destructured.deactivated_application_count, + }; + >::mutate(opening_id, |opening| { - /* - TODO: - Yet another instance of problems due to not following https://github.com/Joystream/joystream/issues/36#issuecomment-539567407 - */ - opening.stage = hiring::OpeningStage::Active { - stage: active_stage, - applicants: applicants, - active_application_count: active_application_count + 1, - unstaking_application_count: unstaking_application_count, - deactivated_application_count: deactivated_application_count, - }; + opening.stage = new_active_stage; }); // DONE - Ok(match success { + Ok(match can_be_added_destructured.would_get_added_success { ApplicationAddedSuccess::CrowdsOutExistingApplication( id_of_croweded_out_application, ) => ApplicationAdded::CrodedOutApplication(id_of_croweded_out_application), @@ -1470,12 +1531,12 @@ impl Module { * Used by `add_application` method. */ -enum ApplicationAddedSuccess { +pub enum ApplicationAddedSuccess { Unconditionally, CrowdsOutExistingApplication(T::ApplicationId), } -enum ApplicationWouldGetAddedEvaluation { +pub enum ApplicationWouldGetAddedEvaluation { No, Yes(ApplicationAddedSuccess), } diff --git a/src/macroes.rs b/src/macroes.rs index 266a25ce01..2953788a7c 100644 --- a/src/macroes.rs +++ b/src/macroes.rs @@ -92,17 +92,17 @@ macro_rules! ensure_opening_is_active { match $stage { hiring::OpeningStage::Active { // <= need proper type here in the future, not param - stage, - applicants, - active_application_count, - unstaking_application_count, - deactivated_application_count, + ref stage, + ref applicants, + ref active_application_count, + ref unstaking_application_count, + ref deactivated_application_count, } => Ok(( - stage, - applicants, - active_application_count, - unstaking_application_count, - deactivated_application_count, + stage.clone(), + applicants.clone(), + active_application_count.clone(), + unstaking_application_count.clone(), + deactivated_application_count.clone(), )), _ => Err($error), } @@ -188,7 +188,40 @@ macro_rules! ensure_active_opening_is_in_review_period { /// /// Returns ... #[macro_export] -macro_rules! ensure_stake_imbalance_matches_staking_policy { +macro_rules! ensure_stake_balance_matches_staking_policy { + ( + $opt_balance:expr, + $opt_policy: expr, + $stake_missing_when_required_error:expr, + $stake_provided_when_redundant_error:expr, + $stake_amount_too_low_error:expr + + ) => {{ + if let Some(ref balance) = $opt_balance { + if let Some(ref policy) = $opt_policy { + + if !policy.accepts_amount(balance) { + Err($stake_amount_too_low_error) + } else { + Ok(Some(balance.clone())) + } + } else { + Err($stake_provided_when_redundant_error) + } + } else if $opt_policy.is_some() { + Err($stake_missing_when_required_error) + } else { + Ok(None) + } + }}; +} + +/* +/// Ensures that optional imbalance matches requirements of optional staking policy +/// +/// Returns ... +#[macro_export] +macro_rules! ensure_stake_balance_matches_staking_policy { ( $opt_imbalance:expr, $opt_policy: expr, @@ -216,6 +249,7 @@ macro_rules! ensure_stake_imbalance_matches_staking_policy { } }}; } +*/ /// Ensures that an optional unstaking period is at least one block whens set. /// From 1fc9ba3691e3bdda226132ae766241e1692beba8 Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Wed, 20 Nov 2019 15:22:14 +0100 Subject: [PATCH 16/25] Added some trait derivations to help in downstream module --- src/lib.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1967064c8b..e4c21778dd 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -269,7 +269,7 @@ decl_module! { } } -#[derive(Debug, PartialEq)] +#[derive(Eq, PartialEq, Clone, Debug)] pub enum StakePurpose { Role, Application, @@ -282,7 +282,7 @@ pub enum ActivateOpeningAt { ExactBlock(BlockNumber), } -#[derive(Debug, PartialEq)] +#[derive(Eq, PartialEq, Clone, Debug)] pub enum AddOpeningError { OpeningMustActivateInTheFuture, @@ -299,12 +299,14 @@ pub struct ApplicationUnstakingPeriods { */ /// The possible outcome for an application in an opening which is being filled. +#[derive(Eq, PartialEq, Clone, Debug)] pub enum ApplicationOutcomeInFilledOpening { Success, Failure, } /// Error due to attempting to fill an opening. +#[derive(Eq, PartialEq, Clone, Debug)] pub enum FillOpeningError { OpeningDoesNotExist, OpeningNotInReviewPeriodStage, @@ -314,6 +316,7 @@ pub enum FillOpeningError { ApplicationNotInActiveStage(T::ApplicationId), } +#[derive(Eq, PartialEq, Clone, Debug)] pub enum CancelOpeningError { UnstakingPeriodTooShort(StakePurpose), RedundantUnstakingPeriodProvided(StakePurpose), @@ -324,20 +327,24 @@ pub enum CancelOpeningError { /// NB: /// `OpeningCancelled` does not have the ideal form. /// https://github.com/Joystream/substrate-hiring-module/issues/10 +#[derive(Eq, PartialEq, Clone, Debug)] pub struct OpeningCancelled { pub number_of_unstaking_applications: u32, pub number_of_deactivated_applications: u32, } +#[derive(Eq, PartialEq, Clone, Debug)] pub enum RemoveOpeningError { OpeningDoesNotExist, } +#[derive(Eq, PartialEq, Clone, Debug)] pub enum BeginReviewError { OpeningDoesNotExist, OpeningNotInAcceptingApplicationsStage, } +#[derive(Eq, PartialEq, Clone, Debug)] pub enum BeginAcceptingApplicationsError { OpeningDoesNotExist, OpeningIsNotInWaitingToBeginStage, @@ -361,6 +368,7 @@ pub struct DestructuredApplicationCanBeAddedEvaluation { } +#[derive(Eq, PartialEq, Clone, Debug)] pub enum AddApplicationError { OpeningDoesNotExist, StakeProvidedWhenRedundant(StakePurpose), @@ -370,11 +378,13 @@ pub enum AddApplicationError { NewApplicationWasCrowdedOut, } +#[derive(Eq, PartialEq, Clone, Debug)] pub enum ApplicationAdded { NoCrowdingOut, CrodedOutApplication(ApplicationId), } +#[derive(Eq, PartialEq, Clone, Debug)] pub enum DeactivateApplicationError { ApplicationDoesNotExist, ApplicationNotActive, From 32026b75b249d004344255ce1ca87084f0a9366c Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Wed, 20 Nov 2019 17:15:22 +0100 Subject: [PATCH 17/25] Expose id of new application added in add_application --- src/lib.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index e4c21778dd..2c8de6fecb 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -379,9 +379,13 @@ pub enum AddApplicationError { } #[derive(Eq, PartialEq, Clone, Debug)] -pub enum ApplicationAdded { - NoCrowdingOut, - CrodedOutApplication(ApplicationId), +pub struct ApplicationAdded { + + /// ... + application_id_added: ApplicationId, + + /// ... + application_id_crowded_out: Option } #[derive(Eq, PartialEq, Clone, Debug)] @@ -1044,12 +1048,17 @@ impl Module { }); // DONE - Ok(match can_be_added_destructured.would_get_added_success { - ApplicationAddedSuccess::CrowdsOutExistingApplication( - id_of_croweded_out_application, - ) => ApplicationAdded::CrodedOutApplication(id_of_croweded_out_application), - _ => ApplicationAdded::NoCrowdingOut, - }) + let application_added_result = ApplicationAdded{ + application_id_added: new_application_id, + application_id_crowded_out: + + match can_be_added_destructured.would_get_added_success { + ApplicationAddedSuccess::CrowdsOutExistingApplication(id) => Some(id), + _ => None, + } + }; + + Ok(application_added_result) } /// Deactive an active application. From c66b2591787e86ec3fecbc9bd358d5beec923299 Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Wed, 20 Nov 2019 17:24:06 +0100 Subject: [PATCH 18/25] Added missed visibility modifiers --- src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 2c8de6fecb..5c320dc0a0 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -382,10 +382,10 @@ pub enum AddApplicationError { pub struct ApplicationAdded { /// ... - application_id_added: ApplicationId, + pub application_id_added: ApplicationId, /// ... - application_id_crowded_out: Option + pub application_id_crowded_out: Option } #[derive(Eq, PartialEq, Clone, Debug)] From f11b610cffa53e254655b5509e06073b0aa0f0e9 Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Fri, 29 Nov 2019 13:02:26 +0100 Subject: [PATCH 19/25] Change name of macro ensure_map_has_mapping_with_key => ensure_map_key --- src/macroes.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/macroes.rs b/src/macroes.rs index 2953788a7c..2636f62d8b 100644 --- a/src/macroes.rs +++ b/src/macroes.rs @@ -15,7 +15,7 @@ macro_rules! ensure_eq { /// /// Returns ... #[macro_export] -macro_rules! ensure_map_has_mapping_with_key { +macro_rules! ensure_map_key { ($map_variable_name:ident , $runtime_trait:tt, $key:expr, $error:expr) => {{ if <$map_variable_name<$runtime_trait>>::exists($key) { let value = <$map_variable_name<$runtime_trait>>::get($key); @@ -35,7 +35,7 @@ macro_rules! ensure_map_has_mapping_with_key { #[macro_export] macro_rules! ensure_opening_exists { ($runtime_trait:tt, $opening_id:expr, $error:expr) => {{ - ensure_map_has_mapping_with_key!(OpeningById, $runtime_trait, $opening_id, $error) + ensure_map_key!(OpeningById, $runtime_trait, $opening_id, $error) }}; } @@ -45,7 +45,7 @@ macro_rules! ensure_opening_exists { ($runtime_trait:tt, $application_id:expr, $error:expr, auto_fetch_opening) => {{ - let application = ensure_map_has_mapping_with_key!( + let application = ensure_map_key!( ApplicationById, $runtime_trait, $application_id, @@ -67,7 +67,7 @@ macro_rules! ensure_opening_exists { #[macro_export] macro_rules! ensure_application_exists { ($runtime_trait:tt, $application_id:expr, $error:expr) => {{ - ensure_map_has_mapping_with_key!(ApplicationById, $runtime_trait, $application_id, $error) + ensure_map_key!(ApplicationById, $runtime_trait, $application_id, $error) }}; ($runtime_trait:tt, $application_id:expr, $error:expr, auto_fetch_opening) => {{ From c9f8c6533eec48e2e526edcaf55f84bd1496dc2c Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Fri, 29 Nov 2019 13:12:59 +0100 Subject: [PATCH 20/25] Use One trait, rather than SimpleArithmetic::from --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 5c320dc0a0..0f76d34e70 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -473,7 +473,7 @@ impl Module { >::insert(new_opening_id, new_opening); // Update NextOpeningId counter - >::mutate(|id| *id += T::OpeningId::from(1)); + >::mutate(|id| *id += T::OpeningId::one()); // Return Ok(new_opening_id) From 3eb105705dd2f317af291e361095a0d7a3822bad Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Fri, 29 Nov 2019 14:47:59 +0100 Subject: [PATCH 21/25] Fix bug in cancel_opening --- src/hiring.rs | 4 +++- src/lib.rs | 64 ++++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 54 insertions(+), 14 deletions(-) diff --git a/src/hiring.rs b/src/hiring.rs index 7e4a0f9eb8..1ede3fb26a 100644 --- a/src/hiring.rs +++ b/src/hiring.rs @@ -138,7 +138,9 @@ pub enum ActiveOpeningStage { started_accepting_applicants_at_block: BlockNumber, - started_review_period_at_block: BlockNumber, + // Whether the review period had ever been started, and if so, at what block. + // Deactivation can also occur directly from the AcceptingApplications stage. + started_review_period_at_block: Option, }, } diff --git a/src/lib.rs b/src/lib.rs index 0f76d34e70..0759f066a4 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -249,7 +249,7 @@ decl_module! { cause: hiring::OpeningDeactivationCause::ReviewPeriodExpired, deactivated_at_block: now, started_accepting_applicants_at_block: started_accepting_applicants_at_block, - started_review_period_at_block: started_review_period_at_block, + started_review_period_at_block: Some(started_review_period_at_block), }, applicants: BTreeSet::new(), @@ -495,23 +495,47 @@ impl Module { let ( active_stage, applicants, - _active_application_count, - _unstaking_application_count, - _deactivated_application_count, + active_application_count, + unstaking_application_count, + deactivated_application_count, ) = ensure_opening_is_active!( opening.stage, CancelOpeningError::OpeningNotInCancellableStage )?; - // NB: Not macroised yet, since this is only place where its used. - ensure!( - match active_stage { - ActiveOpeningStage::AcceptingApplications { .. } - | ActiveOpeningStage::ReviewPeriod { .. } => true, - _ => false, + // + let current_block_height = >::block_number(); // move later! + + let new_active_stage = + match active_stage { + ActiveOpeningStage::AcceptingApplications { started_accepting_applicants_at_block } => { + + Ok( + ActiveOpeningStage::Deactivated { + cause: OpeningDeactivationCause::CancelledAcceptingApplications, + deactivated_at_block: current_block_height, + started_accepting_applicants_at_block: started_accepting_applicants_at_block, + started_review_period_at_block: None, + } + ) + }, - CancelOpeningError::OpeningNotInCancellableStage - ); + ActiveOpeningStage::ReviewPeriod { started_accepting_applicants_at_block, started_review_period_at_block} => { + + Ok( + ActiveOpeningStage::Deactivated { + cause: OpeningDeactivationCause::CancelledInReviewPeriod, + deactivated_at_block: current_block_height, + started_accepting_applicants_at_block: started_accepting_applicants_at_block, + started_review_period_at_block: Some(started_review_period_at_block), + } + ) + + }, + ActiveOpeningStage::Deactivated {..} => { + Err(CancelOpeningError::OpeningNotInCancellableStage) + }, + }?; // Ensure unstaking periods are OK. ensure_opt_unstaking_period_is_ok!( @@ -532,6 +556,20 @@ impl Module { // == MUTATION SAFE == // + // Create and store new cancelled opening + let new_opening = Opening{ + stage: OpeningStage::Active{ + stage: new_active_stage, + applicants: applicants.clone(), + active_application_count: active_application_count, + unstaking_application_count: unstaking_application_count, + deactivated_application_count: deactivated_application_count, + }, + ..opening + }; + + OpeningById::::insert(opening_id, new_opening); + // Map with applications let applications_map = Self::application_id_iter_to_map(applicants.iter()); @@ -834,7 +872,7 @@ impl Module { cause: OpeningDeactivationCause::Filled, deactivated_at_block: current_block_height, started_accepting_applicants_at_block: started_accepting_applicants_at_block, - started_review_period_at_block: started_review_period_at_block, + started_review_period_at_block: Some(started_review_period_at_block), }, //.. <== cant use here, same issue applicants: applicants, From b21f3c8afbcce77d5beda16107bfbcd182c02c5a Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Sat, 30 Nov 2019 11:24:06 +0100 Subject: [PATCH 22/25] Renamed field applicants => applications_added --- src/hiring.rs | 2 +- src/lib.rs | 50 +++++++++++++++++++++++++------------------------- src/macroes.rs | 4 ++-- src/test.rs | 2 +- 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/hiring.rs b/src/hiring.rs index 1ede3fb26a..9655d72d98 100644 --- a/src/hiring.rs +++ b/src/hiring.rs @@ -160,7 +160,7 @@ pub enum OpeningStage { // Set of identifiers for all applications which have been added, but not removed, for this opening. // Is required for timely on-chain lookup of all applications associated with an opening. - applicants: BTreeSet, //BTreeMap, //Vec, + applications_added: BTreeSet, //BTreeMap, //Vec, // TODO: Drop these counters // https://github.com/Joystream/substrate-hiring-module/issues/9 diff --git a/src/lib.rs b/src/lib.rs index 0759f066a4..511bcfd9e5 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -155,7 +155,7 @@ decl_module! { stage: hiring::ActiveOpeningStage::AcceptingApplications { started_accepting_applicants_at_block: now }, - applicants: BTreeSet::new(), + applications_added: BTreeSet::new(), active_application_count: 0, unstaking_application_count: 0, deactivated_application_count: 0 @@ -173,7 +173,7 @@ decl_module! { if let hiring::OpeningStage::Active { ref stage, - ref applicants, + ref applications_added, ref active_application_count, ref unstaking_application_count, ref deactivated_application_count @@ -189,7 +189,7 @@ decl_module! { opening.clone(), ( stage.clone(), - applicants.clone(), + applications_added.clone(), *active_application_count, *unstaking_application_count, *deactivated_application_count, @@ -211,7 +211,7 @@ decl_module! { opening, ( _stage, - applicants, + applications_added, _active_application_count, _unstaking_application_count, _deactivated_application_count, @@ -229,7 +229,7 @@ decl_module! { // Get applications - let applications_map = Self::application_id_iter_to_map(applicants.iter()); + let applications_map = Self::application_id_iter_to_map(applications_added.iter()); // Deactivate Self::initiate_application_deactivations( @@ -252,7 +252,7 @@ decl_module! { started_review_period_at_block: Some(started_review_period_at_block), }, - applicants: BTreeSet::new(), + applications_added: BTreeSet::new(), active_application_count: 0, unstaking_application_count: 0, deactivated_application_count: 0 @@ -356,7 +356,7 @@ pub struct DestructuredApplicationCanBeAddedEvaluation { pub active_stage: ActiveOpeningStage, - pub applicants: BTreeSet, + pub applications_added: BTreeSet, pub active_application_count: u32, @@ -443,7 +443,7 @@ impl Module { }, // Empty set of applicants - applicants: BTreeSet::new(), // Map::new(), + applications_added: BTreeSet::new(), // Map::new(), // All counters set to 0 active_application_count: 0, @@ -494,7 +494,7 @@ impl Module { let ( active_stage, - applicants, + applications_added, active_application_count, unstaking_application_count, deactivated_application_count, @@ -560,7 +560,7 @@ impl Module { let new_opening = Opening{ stage: OpeningStage::Active{ stage: new_active_stage, - applicants: applicants.clone(), + applications_added: applications_added.clone(), active_application_count: active_application_count, unstaking_application_count: unstaking_application_count, deactivated_application_count: deactivated_application_count, @@ -571,7 +571,7 @@ impl Module { OpeningById::::insert(opening_id, new_opening); // Map with applications - let applications_map = Self::application_id_iter_to_map(applicants.iter()); + let applications_map = Self::application_id_iter_to_map(applications_added.iter()); // Initiate deactivation of all active applications let net_result = Self::initiate_application_deactivations( @@ -631,7 +631,7 @@ impl Module { stage: hiring::ActiveOpeningStage::AcceptingApplications { started_accepting_applicants_at_block: current_block_height, }, - applicants: BTreeSet::new(), //BTreeMap::new(), + applications_added: BTreeSet::new(), //BTreeMap::new(), active_application_count: 0, unstaking_application_count: 0, deactivated_application_count: 0, @@ -654,7 +654,7 @@ impl Module { let ( active_stage, - applicants, + applications_added, active_application_count, unstaking_application_count, deactivated_application_count, @@ -680,7 +680,7 @@ impl Module { started_accepting_applicants_at_block: started_accepting_applicants_at_block, started_review_period_at_block: current_block_height, }, - applicants, + applications_added, active_application_count, unstaking_application_count, deactivated_application_count, @@ -710,7 +710,7 @@ impl Module { let ( active_stage, - applicants, + applications_added, active_application_count, unstaking_application_count, deactivated_application_count, @@ -841,7 +841,7 @@ impl Module { // Deactivate all unsuccessful applications, with cause being not being hired. // First get all failed applications by their id. - let failed_applications_map = applicants + let failed_applications_map = applications_added .difference(&successful_applications) .cloned() .map(|application_id| { @@ -875,7 +875,7 @@ impl Module { started_review_period_at_block: Some(started_review_period_at_block), }, //.. <== cant use here, same issue - applicants: applicants, + applications_added: applications_added, active_application_count: active_application_count, unstaking_application_count: unstaking_application_count, deactivated_application_count: deactivated_application_count, @@ -925,7 +925,7 @@ impl Module { let ( active_stage, - applicants, + applications_added, active_application_count, unstaking_application_count, deactivated_application_count, @@ -942,7 +942,7 @@ impl Module { // Ensure that the new application would actually make it let would_get_added_success = ensure_application_would_get_added!( &opening.application_rationing_policy, - &applicants, + &applications_added, &opt_role_stake_balance, &opt_application_stake_balance, AddApplicationError::NewApplicationWasCrowdedOut @@ -952,7 +952,7 @@ impl Module { Ok(DestructuredApplicationCanBeAddedEvaluation{ opening: opening, active_stage: active_stage, - applicants: applicants, + applications_added: applications_added, active_application_count: active_application_count, unstaking_application_count: unstaking_application_count, deactivated_application_count: deactivated_application_count, @@ -1075,7 +1075,7 @@ impl Module { */ let new_active_stage = hiring::OpeningStage::Active { stage: can_be_added_destructured.active_stage, - applicants: can_be_added_destructured.applicants, + applications_added: can_be_added_destructured.applications_added, active_application_count: can_be_added_destructured.active_application_count + 1, unstaking_application_count: can_be_added_destructured.unstaking_application_count, deactivated_application_count: can_be_added_destructured.deactivated_application_count, @@ -1278,7 +1278,7 @@ impl Module { // NB: This ugly byref destructuring is same issue as pointed out multiple times now. if let hiring::OpeningStage::Active { ref stage, - ref applicants, + ref applications_added, active_application_count, unstaking_application_count, deactivated_application_count, @@ -1286,7 +1286,7 @@ impl Module { { opening.stage = hiring::OpeningStage::Active { stage: stage.clone(), - applicants: applicants.clone(), + applications_added: applications_added.clone(), active_application_count: active_application_count, unstaking_application_count: unstaking_application_count - 1, deactivated_application_count: deactivated_application_count + 1, @@ -1464,7 +1464,7 @@ impl Module { // NB: This ugly byref destructuring is same issue as pointed out multiple times now. if let hiring::OpeningStage::Active { ref stage, - ref applicants, + ref applications_added, ref active_application_count, ref unstaking_application_count, ref deactivated_application_count, @@ -1480,7 +1480,7 @@ impl Module { opening.stage = hiring::OpeningStage::Active { stage: (*stage).clone(), // <= truly horrible - applicants: (*applicants).clone(), // <= truly horrible + applications_added: (*applications_added).clone(), // <= truly horrible active_application_count: new_active_application_count, unstaking_application_count: new_unstaking_application_count, deactivated_application_count: new_deactivated_application_count, diff --git a/src/macroes.rs b/src/macroes.rs index 2636f62d8b..5053a3ac4e 100644 --- a/src/macroes.rs +++ b/src/macroes.rs @@ -93,13 +93,13 @@ macro_rules! ensure_opening_is_active { hiring::OpeningStage::Active { // <= need proper type here in the future, not param ref stage, - ref applicants, + ref applications_added, ref active_application_count, ref unstaking_application_count, ref deactivated_application_count, } => Ok(( stage.clone(), - applicants.clone(), + applications_added.clone(), active_application_count.clone(), unstaking_application_count.clone(), deactivated_application_count.clone(), diff --git a/src/test.rs b/src/test.rs index e6d731ba54..9762afce69 100644 --- a/src/test.rs +++ b/src/test.rs @@ -52,7 +52,7 @@ fn add_opening_success_waiting_to_begin() { stage: ActiveOpeningStage::AcceptingApplications { started_accepting_applicants_at_block: FIRST_BLOCK_HEIGHT }, - applicants: BTreeSet::new(), + applications_added: BTreeSet::new(), active_application_count: 0, unstaking_application_count: 0, deactivated_application_count: 0 From 9f886343970984e621d628bef4e089a187034a30 Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Sat, 30 Nov 2019 12:22:35 +0100 Subject: [PATCH 23/25] Fixed bug: incorrect staking purpose --- src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 511bcfd9e5..063763821c 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -768,11 +768,11 @@ impl Module { opt_failed_applicant_role_stake_unstaking_period, opening.role_staking_policy, FillOpeningError::UnstakingPeriodTooShort( - StakePurpose::Application, + StakePurpose::Role, ApplicationOutcomeInFilledOpening::Failure ), FillOpeningError::RedundantUnstakingPeriodProvided( - StakePurpose::Application, + StakePurpose::Role, ApplicationOutcomeInFilledOpening::Failure ) )?; From e95122d7f29bdf0874ac584067be83c8d5fca59c Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Sat, 30 Nov 2019 13:05:59 +0100 Subject: [PATCH 24/25] Adding assert for deactivation of applications --- src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 063763821c..e1bb8cc5d5 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -1470,6 +1470,9 @@ impl Module { ref deactivated_application_count, } = opening.stage { + + assert!(*active_application_count > 0); + let new_active_application_count = active_application_count - 1; let new_unstaking_application_count = From 5519a69984ca52a9198a1f9f5be304d5153859bd Mon Sep 17 00:00:00 2001 From: Bedeho Mender Date: Sat, 30 Nov 2019 13:14:22 +0100 Subject: [PATCH 25/25] Drop dangling rustdoc comment --- src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index e1bb8cc5d5..a29d64e026 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -1531,8 +1531,6 @@ impl Module { opt_stake_id.is_some() } - /// TODO: these also look messy - fn opt_staking_policy_to_crowded_out_unstaking_period( opt_staking_policy: &Option, T::BlockNumber>>, ) -> Option {