council - winner vote stakes longer

runtime-modules/council/src/lib.rs

@@ -59,7 +59,7 @@ use sp_runtime::traits::{Hash, MaybeSerialize, Member};
 use std::marker::PhantomData;
 use system::ensure_signed;
 
-use referendum::{OptionResult, ReferendumManager};
+use referendum::{CastVote, OptionResult, ReferendumManager};
 
 // declared modules
 mod mock;
@@ -152,6 +152,7 @@ pub type VotePowerOf<T> = <<T as Trait>::Referendum as ReferendumManager<
     <T as system::Trait>::AccountId,
     <T as system::Trait>::Hash,
 >>::VotePower;
+pub type CastVoteOf<T> = CastVote<<T as system::Trait>::Hash, Balance<T>>;
 
 pub type CouncilMemberOf<T> =
     CouncilMember<<T as system::Trait>::AccountId, <T as Trait>::MembershipId, Balance<T>>;
@@ -175,7 +176,8 @@ pub trait Trait: system::Trait {
         + Copy
         + MaybeSerialize
         + PartialEq
-        + From<u64>;
+        + From<u64>
+        + Into<u64>;
 
     /// Referendum used for council elections.
     type Referendum: ReferendumManager<Self::Origin, Self::AccountId, Self::Hash>;
@@ -212,7 +214,10 @@ pub trait ReferendumConnection<T: Trait> {
         -> Result<(), Error<T>>;
 
     /// Process referendum results. This function MUST be called in runtime's implementation of referendum's `can_release_voting_stake()`.
-    fn can_release_vote_stake() -> Result<(), Error<T>>;
+    fn can_release_vote_stake(
+        vote: &CastVoteOf<T>,
+        current_voting_cycle_id: &u64,
+    ) -> Result<(), Error<T>>;
 
     /// Checks that user is indeed candidating. This function MUST be called in runtime's implementation of referendum's `is_valid_option_id()`.
     fn is_valid_candidate_id(membership_id: &T::MembershipId) -> bool;
@@ -552,12 +557,35 @@ impl<T: Trait> ReferendumConnection<T> for Module<T> {
     }
 
     /// Check that it is a proper time to release stake.
-    fn can_release_vote_stake() -> Result<(), Error<T>> {
-        // ensure it's proper time to release stake
-        match Stage::<T>::get().stage {
-            CouncilStage::Idle => (),
-            _ => return Err(Error::CantReleaseStakeNow),
-        };
+    fn can_release_vote_stake(
+        vote: &CastVoteOf<T>,
+        current_voting_cycle_id: &u64,
+    ) -> Result<(), Error<T>> {
+        // allow release for very old votes
+        if vote.cycle_id > current_voting_cycle_id + 1 {
+            return Ok(());
+        }
+
+        let voting_for_winner = CouncilMembers::<T>::get()
+            .iter()
+            .map(|council_member| council_member.membership_id)
+            .any(|membership_id| vote.vote_for == Some(membership_id.into()));
+
+        // allow release for vote from previous elections only when not voted for winner
+        if vote.cycle_id == current_voting_cycle_id + 1 {
+            // ensure vote was not cast for the one of winning candidates / council members
+            if voting_for_winner {
+                return Err(Error::CantReleaseStakeNow);
+            }
+
+            return Ok(());
+        }
+
+        // at this point vote.cycle_id == current_voting_cycle_id
+
+        if voting_for_winner {
+            return Err(Error::CantReleaseStakeNow);
+        }
 
         Ok(())
     }

runtime-modules/council/src/mock.rs

@@ -202,15 +202,20 @@ impl referendum::Trait<ReferendumInstance> for RuntimeReferendum {
         stake
     }
 
-    fn can_release_voting_stake(
-        _vote: &CastVote<Self::Hash, BalanceReferendum<Self, ReferendumInstance>>,
+    fn can_release_vote_stake(
+        vote: &CastVote<Self::Hash, BalanceReferendum<Self, ReferendumInstance>>,
+        current_voting_cycle_id: &u64,
     ) -> bool {
         // trigger fail when requested to do so
         if !IS_UNSTAKE_ENABLED.with(|value| value.borrow().0) {
             return false;
         }
 
-        <Module<Runtime> as ReferendumConnection<Runtime>>::can_release_vote_stake().is_ok()
+        <Module<Runtime> as ReferendumConnection<Runtime>>::can_release_vote_stake(
+            vote,
+            current_voting_cycle_id,
+        )
+        .is_ok()
     }
 
     fn process_results(winners: &[OptionResult<Self::VotePower>]) {
@@ -337,6 +342,9 @@ pub struct CouncilSettings<T: Trait> {
     pub voting_stage_duration: T::BlockNumber,
     pub reveal_stage_duration: T::BlockNumber,
     pub idle_stage_duration: T::BlockNumber,
+
+    pub election_duration: T::BlockNumber,
+    pub cycle_duration: T::BlockNumber,
 }
 
 impl<T: Trait> CouncilSettings<T>
@@ -346,16 +354,32 @@ where
     pub fn extract_settings() -> CouncilSettings<T> {
         let council_size = T::CouncilSize::get();
 
+        let reveal_stage_duration =
+            <RuntimeReferendum as referendum::Trait<ReferendumInstance>>::RevealStageDuration::get(
+            )
+            .into();
+        let announcing_stage_duration = <T as Trait>::AnnouncingPeriodDuration::get();
+        let voting_stage_duration =
+            <RuntimeReferendum as referendum::Trait<ReferendumInstance>>::VoteStageDuration::get()
+                .into();
+        let idle_stage_duration = <T as Trait>::IdlePeriodDuration::get();
+
         CouncilSettings {
             council_size,
             min_candidate_count: council_size + <T as Trait>::MinNumberOfExtraCandidates::get(),
             min_candidate_stake: T::MinCandidateStake::get(),
-            announcing_stage_duration: <T as Trait>::AnnouncingPeriodDuration::get(),
-            voting_stage_duration:
-                <RuntimeReferendum as referendum::Trait<ReferendumInstance>>::VoteStageDuration::get().into(),
-            reveal_stage_duration:
-                <RuntimeReferendum as referendum::Trait<ReferendumInstance>>::RevealStageDuration::get().into(),
+            announcing_stage_duration,
+            voting_stage_duration,
+            reveal_stage_duration,
             idle_stage_duration: <T as Trait>::IdlePeriodDuration::get(),
+
+            election_duration: reveal_stage_duration
+                + announcing_stage_duration
+                + voting_stage_duration,
+            cycle_duration: reveal_stage_duration
+                + announcing_stage_duration
+                + voting_stage_duration
+                + idle_stage_duration,
         }
     }
 }
@@ -720,6 +744,20 @@ where
         );
     }
 
+    pub fn release_vote_stake(
+        origin: OriginType<<Runtime as system::Trait>::AccountId>,
+        expected_result: Result<(), ()>,
+    ) -> () {
+        // check method returns expected result
+        assert_eq!(
+            referendum::Module::<RuntimeReferendum, ReferendumInstance>::release_vote_stake(
+                InstanceMockUtils::<Runtime>::mock_origin(origin),
+            )
+            .is_ok(),
+            expected_result.is_ok(),
+        );
+    }
+
     /// simulate one council's election cycle
     pub fn simulate_council_cycle(params: CouncilCycleParams<T>) {
         let settings = params.council_settings;
@@ -833,7 +871,11 @@ where
     }
 
     /// Simulate one full round of council lifecycle (announcing, election, idle). Use it to quickly test behavior in 2nd, 3rd, etc. cycle.
-    pub fn run_full_council_cycle(start_block_number: T::BlockNumber) -> CouncilCycleParams<T> {
+    pub fn run_full_council_cycle(
+        start_block_number: T::BlockNumber,
+        expected_initial_council_members: &[CouncilMemberOf<T>],
+        users_offset: u64,
+    ) -> CouncilCycleParams<T> {
         let council_settings = CouncilSettings::<T>::extract_settings();
         let vote_stake =
             <RuntimeReferendum as referendum::Trait<ReferendumInstance>>::MinimumStake::get();
@@ -843,7 +885,7 @@ where
             as u64)
             .map(|i| {
                 InstanceMockUtils::<T>::generate_candidate(
-                    u64::from(i),
+                    u64::from(i) + users_offset,
                     council_settings.min_candidate_stake,
                 )
             })
@@ -866,9 +908,9 @@ where
         let voters = (0..votes_map.len())
             .map(|index| {
                 InstanceMockUtils::<T>::generate_voter(
-                    index as u64,
+                    index as u64 + users_offset,
                     vote_stake.into(),
-                    CANDIDATE_BASE_ID + votes_map[index],
+                    CANDIDATE_BASE_ID + votes_map[index] + users_offset,
                 )
             })
             .collect();
@@ -876,7 +918,7 @@ where
         let params = CouncilCycleParams {
             council_settings: CouncilSettings::<T>::extract_settings(),
             cycle_start_block_number: start_block_number,
-            expected_initial_council_members: vec![],
+            expected_initial_council_members: expected_initial_council_members.to_vec(),
             expected_final_council_members,
             candidates_announcing: candidates.clone(),
             expected_candidates,

runtime-modules/council/src/tests.rs

@@ -18,7 +18,7 @@ fn council_lifecycle() {
     let config = default_genesis_config();
 
     build_test_externalities(config).execute_with(|| {
-        Mocks::run_full_council_cycle(0);
+        Mocks::run_full_council_cycle(0, &vec![], 0);
     });
 }
 
@@ -150,6 +150,43 @@ fn council_can_vote_for_yourself() {
     });
 }
 
+/// Test that vote for a succesfull candidate has it's stake locked until the one referendum cycle with succesfull council election
+#[test]
+fn council_vote_for_winner_stakes_longer() {
+    let config = default_genesis_config();
+
+    build_test_externalities(config).execute_with(|| {
+        let council_settings = CouncilSettings::<Runtime>::extract_settings();
+
+        // run first election round
+        let params = Mocks::run_full_council_cycle(0, &vec![], 0);
+        let second_round_user_offset = 100; // some number higher than the number of voters
+
+        let voter_for_winner = params.voters[0].clone();
+        let voter_for_looser = params.voters[params.voters.len() - 1].clone();
+
+        // try to release vote stake
+        Mocks::release_vote_stake(voter_for_winner.origin.clone(), Err(()));
+        Mocks::release_vote_stake(voter_for_looser.origin.clone(), Ok(()));
+
+        // forward past idle stage
+        MockUtils::increase_block_number(council_settings.idle_stage_duration + 1);
+
+        // try to release vote stake
+        Mocks::release_vote_stake(voter_for_winner.origin.clone(), Err(()));
+
+        // run second election round
+        Mocks::run_full_council_cycle(
+            council_settings.cycle_duration,
+            &params.expected_final_council_members,
+            second_round_user_offset,
+        );
+
+        // try to release vote stake
+        Mocks::release_vote_stake(voter_for_winner.origin.clone(), Ok(()));
+    });
+}
+
 // Test that only valid members can candidate.
 #[test]
 fn council_candidacy_invalid_member() {
@@ -675,7 +712,7 @@ fn council_member_stake_automaticly_unlocked() {
             <RuntimeReferendum as referendum::Trait<ReferendumInstance>>::MinimumStake::get();
         let not_reelected_candidate_index = 0;
 
-        let params = Mocks::run_full_council_cycle(0);
+        let params = Mocks::run_full_council_cycle(0, &vec![], 0);
 
         let candidates = params.candidates_announcing.clone();
 
@@ -851,7 +888,7 @@ fn council_repeated_candidacy_unstakes() {
         let not_elected_candidate_index = 2;
 
         // run one council cycle
-        let params = Mocks::run_full_council_cycle(0);
+        let params = Mocks::run_full_council_cycle(0, &vec![], 0);
 
         // forward to next candidacy announcing period
         MockUtils::increase_block_number(council_settings.idle_stage_duration + 1);

runtime-modules/referendum/src/lib.rs

@@ -23,7 +23,7 @@
 //!
 //! - [vote](./struct.Module.html#method.vote)
 //! - [reveal_vote](./struct.Module.html#method.reveal_vote)
-//! - [release_voting_stake](./struct.Module.html#method.release_voting_stake)
+//! - [release_vote_stake](./struct.Module.html#method.release_vote_stake)
 //!
 //! ## Notes
 //! This module is instantiable pallet as described here https://substrate.dev/recipes/3-entrees/instantiable.html
@@ -71,8 +71,8 @@ impl<BlockNumber, VotePower: Encode + Decode> Default for ReferendumStage<BlockN
 /// Representation for voting stage state.
 #[derive(Encode, Decode, PartialEq, Eq, Debug, Default)]
 pub struct ReferendumStageVoting<BlockNumber> {
-    started: BlockNumber,      // block in which referendum started
-    winning_target_count: u64, // target number of winners
+    pub started: BlockNumber,      // block in which referendum started
+    pub winning_target_count: u64, // target number of winners
 }
 
 /// Representation for revealing stage state.
@@ -92,10 +92,10 @@ pub struct OptionResult<VotePower> {
 /// Vote cast in referendum. Vote target is concealed until user reveals commitment's proof.
 #[derive(Encode, Decode, PartialEq, Eq, Debug, Default)]
 pub struct CastVote<Hash, Currency> {
-    commitment: Hash, // a commitment that a user submits in the voting stage before revealing what this vote is actually for
-    cycle_id: u64,    // current referendum cycle number
-    stake: Currency,  // stake locked for vote
-    vote_for: Option<u64>, // target option this vote favors; is `None` before the vote is revealed
+    pub commitment: Hash, // a commitment that a user submits in the voting stage before revealing what this vote is actually for
+    pub cycle_id: u64,    // current referendum cycle number
+    pub stake: Currency,  // stake locked for vote
+    pub vote_for: Option<u64>, // target option this vote favors; is `None` before the vote is revealed
 }
 
 /////////////////// Type aliases ///////////////////////////////////////////////
@@ -194,7 +194,10 @@ pub trait Trait<I: Instance>: system::Trait {
 
     /// Checks if user can unlock his stake from the given vote.
     /// Gives runtime an ability to penalize user for not revealing stake, etc.
-    fn can_release_voting_stake(vote: &CastVote<Self::Hash, Balance<Self, I>>) -> bool;
+    fn can_release_vote_stake(
+        vote: &CastVote<Self::Hash, Balance<Self, I>>,
+        current_voting_cycle_id: &u64,
+    ) -> bool;
 
     /// Gives runtime an ability to react on referendum result.
     fn process_results(winners: &[OptionResult<Self::VotePower>]);
@@ -367,15 +370,15 @@ decl_module! {
 
         /// Release a locked stake.
         #[weight = 10_000_000]
-        pub fn release_voting_stake(origin) -> Result<(), Error<T, I>> {
-            let account_id = EnsureChecks::<T, I>::can_release_voting_stake(origin)?;
+        pub fn release_vote_stake(origin) -> Result<(), Error<T, I>> {
+            let account_id = EnsureChecks::<T, I>::can_release_vote_stake(origin)?;
 
             //
             // == MUTATION SAFE ==
             //
 
             // reveal the vote - it can return error when stake fails to unlock
-            Mutations::<T, I>::release_voting_stake(&account_id);
+            Mutations::<T, I>::release_vote_stake(&account_id);
 
             // emit event
             Self::deposit_event(RawEvent::StakeReleased(account_id));
@@ -607,7 +610,7 @@ impl<T: Trait<I>, I: Instance> Mutations<T, I> {
     }
 
     /// Release stake associated to the user's last vote.
-    fn release_voting_stake(account_id: &<T as system::Trait>::AccountId) {
+    fn release_vote_stake(account_id: &<T as system::Trait>::AccountId) {
         // lock stake amount
         T::Currency::remove_lock(T::LockId::get(), account_id);
 
@@ -820,7 +823,7 @@ impl<T: Trait<I>, I: Instance> EnsureChecks<T, I> {
         Ok((stage_data, account_id, cast_vote))
     }
 
-    fn can_release_voting_stake(origin: T::Origin) -> Result<T::AccountId, Error<T, I>> {
+    fn can_release_vote_stake(origin: T::Origin) -> Result<T::AccountId, Error<T, I>> {
         let cycle_id = CurrentCycleId::<I>::get();
 
         // ensure superuser requested action
@@ -834,7 +837,7 @@ impl<T: Trait<I>, I: Instance> EnsureChecks<T, I> {
         }
 
         // ask runtime if stake can be released
-        if !T::can_release_voting_stake(&cast_vote) {
+        if !T::can_release_vote_stake(&cast_vote, &cycle_id) {
             return Err(Error::UnstakingForbidden);
         }
 

runtime-modules/referendum/src/mock.rs

@@ -91,7 +91,10 @@ impl Trait<Instance0> for Runtime {
         stake
     }
 
-    fn can_release_voting_stake(_vote: &CastVote<Self::Hash, Balance<Self, Instance0>>) -> bool {
+    fn can_release_vote_stake(
+        _vote: &CastVote<Self::Hash, Balance<Self, Instance0>>,
+        _current_voting_cycle_id: &u64,
+    ) -> bool {
         // trigger fail when requested to do so
         if !IS_UNSTAKE_ENABLED.with(|value| value.borrow().0) {
             return false;
@@ -556,10 +559,9 @@ impl InstanceMocks<Runtime, Instance0> {
     ) -> () {
         // check method returns expected result
         assert_eq!(
-            Module::<Runtime, Instance0>::release_voting_stake(InstanceMockUtils::<
-                Runtime,
-                Instance0,
-            >::mock_origin(origin),),
+            Module::<Runtime, Instance0>::release_vote_stake(
+                InstanceMockUtils::<Runtime, Instance0>::mock_origin(origin),
+            ),
             expected_result,
         );