Refactor proposal stakes

- move proposal stake data from the proposal to its statuses

.travis.yml

@@ -1,7 +1,7 @@
 language: rust
 
 rust:
-  - 1.41.1
+  - 1.42.0
 # Caching saves a lot of time but often causes stalled builds...
 # disabled for now
 # look into solution here: https://levans.fr/rust_travis_cache.html

runtime-modules/proposals/engine/src/lib.rs

@@ -44,7 +44,7 @@ use rstd::prelude::*;
 use sr_primitives::traits::{DispatchResult, Zero};
 use srml_support::traits::{Currency, Get};
 use srml_support::{
-    decl_error, decl_event, decl_module, decl_storage, ensure, Parameter, StorageDoubleMap,
+    decl_error, decl_event, decl_module, decl_storage, ensure, print, Parameter, StorageDoubleMap,
 };
 use system::{ensure_root, RawOrigin};
 
@@ -106,6 +106,8 @@ decl_event!(
         <T as Trait>::ProposalId,
         MemberId = MemberId<T>,
         <T as system::Trait>::BlockNumber,
+        <T as system::Trait>::AccountId,
+        <T as stake::Trait>::StakeId,
     {
     	/// Emits on proposal creation.
         /// Params:
@@ -117,7 +119,7 @@ decl_event!(
         /// Params:
         /// - Id of a updated proposal.
         /// - New proposal status
-        ProposalStatusUpdated(ProposalId, ProposalStatus<BlockNumber>),
+        ProposalStatusUpdated(ProposalId, ProposalStatus<BlockNumber, StakeId, AccountId>),
 
         /// Emits on voting for the proposal
         /// Params:
@@ -236,7 +238,7 @@ decl_module! {
             ensure!(<Proposals<T>>::exists(proposal_id), Error::ProposalNotFound);
             let mut proposal = Self::proposals(proposal_id);
 
-            ensure!(proposal.status == ProposalStatus::Active, Error::ProposalFinalized);
+            ensure!(matches!(proposal.status, ProposalStatus::Active{..}), Error::ProposalFinalized);
 
             let did_not_vote_before = !<VoteExistsByProposalByVoter<T>>::exists(
                 proposal_id,
@@ -265,7 +267,7 @@ decl_module! {
             let proposal = Self::proposals(proposal_id);
 
             ensure!(proposer_id == proposal.proposer_id, Error::NotAuthor);
-            ensure!(proposal.status == ProposalStatus::Active, Error::ProposalFinalized);
+            ensure!(matches!(proposal.status, ProposalStatus::Active{..}), Error::ProposalFinalized);
 
             // mutation
 
@@ -279,7 +281,7 @@ decl_module! {
             ensure!(<Proposals<T>>::exists(proposal_id), Error::ProposalNotFound);
             let proposal = Self::proposals(proposal_id);
 
-            ensure!(proposal.status == ProposalStatus::Active, Error::ProposalFinalized);
+            ensure!(matches!(proposal.status, ProposalStatus::Active{..}), Error::ProposalFinalized);
 
             // mutation
 
@@ -360,9 +362,8 @@ impl<T: Trait> Module<T> {
             title,
             description,
             proposer_id: proposer_id.clone(),
-            status: ProposalStatus::Active,
+            status: ProposalStatus::Active(stake_data),
             voting_results: VotingResults::default(),
-            stake_data,
         };
 
         <Proposals<T>>::insert(proposal_id, new_proposal);
@@ -444,12 +445,14 @@ impl<T: Trait> Module<T> {
             if <Proposals<T>>::exists(proposal_id) {
                 let proposal = Self::proposals(proposal_id);
 
-                if let Some(stake_data) = proposal.stake_data {
-                    //TODO: handle the result
-                    let _ = CurrencyOf::<T>::resolve_into_existing(
-                        &stake_data.source_account_id,
-                        imbalance,
-                    );
+                if let ProposalStatus::Active(active_stake_result) = proposal.status {
+                    if let Some(active_stake) = active_stake_result {
+                        //TODO: handle the result
+                        let _ = CurrencyOf::<T>::resolve_into_existing(
+                            &active_stake.source_account_id,
+                            imbalance,
+                        );
+                    }
                 }
             }
         }
@@ -537,31 +540,31 @@ impl<T: Trait> Module<T> {
 
         let mut proposal = Self::proposals(proposal_id);
 
-        if let ProposalDecisionStatus::Approved { .. } = decision_status {
-            <PendingExecutionProposalIds<T>>::insert(proposal_id, ());
-        }
-
-        // deal with stakes if necessary
-        let slash_balance = Self::calculate_slash_balance(&decision_status, &proposal.parameters);
-        let slash_and_unstake_result =
-            Self::slash_and_unstake(proposal.stake_data.clone(), slash_balance);
+        if let ProposalStatus::Active(active_stake) = proposal.status.clone() {
+            if let ProposalDecisionStatus::Approved { .. } = decision_status {
+                <PendingExecutionProposalIds<T>>::insert(proposal_id, ());
+            }
 
-        //TODO: leave stake data as is?
-        if slash_and_unstake_result.is_ok() {
-            proposal.stake_data = None;
-        }
+            // deal with stakes if necessary
+            let slash_balance =
+                Self::calculate_slash_balance(&decision_status, &proposal.parameters);
+            let slash_and_unstake_result =
+                Self::slash_and_unstake(active_stake.clone(), slash_balance);
 
-        // create finalized proposal status with error if any
-        let new_proposal_status = //TODO rename without an error
-            ProposalStatus::finalized_with_error(decision_status, slash_and_unstake_result.err(), Self::current_block());
+            // create finalized proposal status with error if any
+            let new_proposal_status = //TODO rename without an error
+            ProposalStatus::finalized_with_error(decision_status, slash_and_unstake_result.err(), active_stake, Self::current_block());
 
-        proposal.status = new_proposal_status.clone();
-        <Proposals<T>>::insert(proposal_id, proposal);
+            proposal.status = new_proposal_status.clone();
+            <Proposals<T>>::insert(proposal_id, proposal);
 
-        Self::deposit_event(RawEvent::ProposalStatusUpdated(
-            proposal_id,
-            new_proposal_status,
-        ));
+            Self::deposit_event(RawEvent::ProposalStatusUpdated(
+                proposal_id,
+                new_proposal_status,
+            ));
+        } else {
+            print("Broken invariant: proposal cannot be non-active during the finalisation");
+        }
     }
 
     // Slashes the stake and perform unstake only in case of existing stake

runtime-modules/proposals/engine/src/tests/mod.rs

@@ -243,7 +243,7 @@ impl VoteGenerator {
 
 struct EventFixture;
 impl EventFixture {
-    fn assert_events(expected_raw_events: Vec<RawEvent<u32, u64, u64>>) {
+    fn assert_events(expected_raw_events: Vec<RawEvent<u32, u64, u64, u64, u64>>) {
         let expected_events = expected_raw_events
             .iter()
             .map(|ev| EventRecord {
@@ -340,7 +340,6 @@ fn proposal_execution_succeeds() {
                     rejections: 0,
                     slashes: 0,
                 },
-                stake_data: None,
             }
         );
 
@@ -393,7 +392,6 @@ fn proposal_execution_failed() {
                     rejections: 0,
                     slashes: 0,
                 },
-                stake_data: None,
             }
         )
     });
@@ -572,7 +570,6 @@ fn cancel_proposal_succeeds() {
                 title: b"title".to_vec(),
                 description: b"description".to_vec(),
                 voting_results: VotingResults::default(),
-                stake_data: None,
             }
         )
     });
@@ -641,7 +638,6 @@ fn veto_proposal_succeeds() {
                 title: b"title".to_vec(),
                 description: b"description".to_vec(),
                 voting_results: VotingResults::default(),
-                stake_data: None,
             }
         );
 
@@ -727,6 +723,7 @@ fn cancel_proposal_event_emitted() {
                 ProposalStatus::Finalized(FinalizationData {
                     proposal_status: ProposalDecisionStatus::Canceled,
                     encoded_unstaking_error_due_to_broken_runtime: None,
+                    stake_data_after_unstaking_error: None,
                     finalized_at: 1,
                 }),
             ),
@@ -772,7 +769,6 @@ fn create_proposal_and_expire_it() {
                 title: b"title".to_vec(),
                 description: b"description".to_vec(),
                 voting_results: VotingResults::default(),
-                stake_data: None,
             }
         )
     });
@@ -818,7 +814,6 @@ fn proposal_execution_postponed_because_of_grace_period() {
                     rejections: 0,
                     slashes: 0,
                 },
-                stake_data: None,
             }
         );
     });
@@ -860,7 +855,6 @@ fn proposal_execution_succeeds_after_the_grace_period() {
                 rejections: 0,
                 slashes: 0,
             },
-            stake_data: None,
         };
 
         assert_eq!(proposal, expected_proposal);
@@ -955,14 +949,13 @@ fn create_dummy_proposal_succeeds_with_stake() {
                 parameters: parameters_fixture.params(),
                 proposer_id: 1,
                 created_at: 1,
-                status: ProposalStatus::Active,
+                status: ProposalStatus::Active(Some(ActiveStake {
+                    stake_id: 0, // valid stake_id
+                    source_account_id: 1
+                })),
                 title: b"title".to_vec(),
                 description: b"description".to_vec(),
                 voting_results: VotingResults::default(),
-                stake_data: Some(ActiveStake {
-                    stake_id: 0, // valid stake_id
-                    source_account_id: 1
-                }),
             }
         )
     });
@@ -1162,6 +1155,7 @@ fn proposal_slashing_succeeds() {
                 proposal_status: ProposalDecisionStatus::Slashed,
                 encoded_unstaking_error_due_to_broken_runtime: None,
                 finalized_at: 1,
+                stake_data_after_unstaking_error: None,
             }),
         );
         assert!(!<ActiveProposalIds<Test>>::exists(proposal_id));
@@ -1218,15 +1212,15 @@ fn finalize_proposal_using_stake_mocks_failed() {
                     status: ProposalStatus::finalized_with_error(
                         ProposalDecisionStatus::Expired,
                         Some("Cannot remove stake"),
+                        Some(ActiveStake {
+                            stake_id: 1,
+                            source_account_id: 1
+                        }),
                         4,
                     ),
                     title: b"title".to_vec(),
                     description: b"description".to_vec(),
                     voting_results: VotingResults::default(),
-                    stake_data: Some(ActiveStake {
-                        stake_id: 1,
-                        source_account_id: 1
-                    }),
                 }
             );
         });

runtime-modules/proposals/engine/src/types/mod.rs

@@ -142,19 +142,18 @@ pub struct Proposal<BlockNumber, ProposerId, Balance, StakeId, AccountId> {
     pub created_at: BlockNumber,
 
     /// Current proposal status
-    pub status: ProposalStatus<BlockNumber>,
+    pub status: ProposalStatus<BlockNumber, StakeId, AccountId>,
 
     /// Curring voting result for the proposal
     pub voting_results: VotingResults,
-
-    /// Stake data for the proposal
-    pub stake_data: Option<ActiveStake<StakeId, AccountId>>,
 }
 
 impl<BlockNumber, ProposerId, Balance, StakeId, AccountId>
     Proposal<BlockNumber, ProposerId, Balance, StakeId, AccountId>
 where
     BlockNumber: Add<Output = BlockNumber> + PartialOrd + Copy,
+    StakeId: Clone,
+    AccountId: Clone,
 {
     /// Returns whether voting period expired by now
     pub fn is_voting_period_expired(&self, now: BlockNumber) -> bool {
@@ -233,6 +232,8 @@ impl<'a, BlockNumber, ProposerId, Balance, StakeId, AccountId>
     ProposalStatusResolution<'a, BlockNumber, ProposerId, Balance, StakeId, AccountId>
 where
     BlockNumber: Add<Output = BlockNumber> + PartialOrd + Copy,
+    StakeId: Clone,
+    AccountId: Clone,
 {
     // Proposal has been expired and quorum not reached.
     pub fn is_expired(&self) -> bool {
@@ -353,7 +354,7 @@ pub(crate) struct ApprovedProposalData<
     pub proposal: Proposal<BlockNumber, ProposerId, Balance, StakeId, AccountId>,
 
     /// Proposal finalisation status data
-    pub finalisation_status_data: FinalizationData<BlockNumber>,
+    pub finalisation_status_data: FinalizationData<BlockNumber, StakeId, AccountId>,
 }
 
 #[cfg(test)]

runtime-modules/proposals/engine/src/types/proposal_statuses.rs

@@ -1,46 +1,49 @@
 use codec::{Decode, Encode};
 use rstd::prelude::*;
 
+use crate::ActiveStake;
 #[cfg(feature = "std")]
 use serde::{Deserialize, Serialize};
 
 /// Current status of the proposal
 #[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
 #[derive(Encode, Decode, Clone, PartialEq, Eq, Debug)]
-pub enum ProposalStatus<BlockNumber> {
-    /// A new proposal that is available for voting.
-    Active,
+pub enum ProposalStatus<BlockNumber, StakeId, AccountId> {
+    /// A new proposal status that is available for voting (with optional stake data).
+    Active(Option<ActiveStake<StakeId, AccountId>>),
 
     /// The proposal decision was made.
-    Finalized(FinalizationData<BlockNumber>),
+    Finalized(FinalizationData<BlockNumber, StakeId, AccountId>),
 }
 
-impl<BlockNumber> Default for ProposalStatus<BlockNumber> {
+impl<BlockNumber, StakeId, AccountId> Default for ProposalStatus<BlockNumber, StakeId, AccountId> {
     fn default() -> Self {
-        ProposalStatus::Active
+        ProposalStatus::Active(None)
     }
 }
 
-impl<BlockNumber> ProposalStatus<BlockNumber> {
+impl<BlockNumber, StakeId, AccountId> ProposalStatus<BlockNumber, StakeId, AccountId> {
     /// Creates finalized proposal status with provided ProposalDecisionStatus
     pub fn finalized(
         decision_status: ProposalDecisionStatus,
         now: BlockNumber,
-    ) -> ProposalStatus<BlockNumber> {
-        Self::finalized_with_error(decision_status, None, now)
+    ) -> ProposalStatus<BlockNumber, StakeId, AccountId> {
+        Self::finalized_with_error(decision_status, None, None, now)
     }
 
     /// Creates finalized proposal status with provided ProposalDecisionStatus and error
     pub fn finalized_with_error(
         decision_status: ProposalDecisionStatus,
         encoded_unstaking_error_due_to_broken_runtime: Option<&str>,
+        active_stake: Option<ActiveStake<StakeId, AccountId>>,
         now: BlockNumber,
-    ) -> ProposalStatus<BlockNumber> {
+    ) -> ProposalStatus<BlockNumber, StakeId, AccountId> {
         ProposalStatus::Finalized(FinalizationData {
             proposal_status: decision_status,
             encoded_unstaking_error_due_to_broken_runtime:
                 encoded_unstaking_error_due_to_broken_runtime.map(|err| err.as_bytes().to_vec()),
             finalized_at: now,
+            stake_data_after_unstaking_error: active_stake,
         })
     }
 
@@ -48,11 +51,12 @@ impl<BlockNumber> ProposalStatus<BlockNumber> {
     pub fn approved(
         approved_status: ApprovedProposalStatus,
         now: BlockNumber,
-    ) -> ProposalStatus<BlockNumber> {
+    ) -> ProposalStatus<BlockNumber, StakeId, AccountId> {
         ProposalStatus::Finalized(FinalizationData {
             proposal_status: ProposalDecisionStatus::Approved(approved_status),
             encoded_unstaking_error_due_to_broken_runtime: None,
             finalized_at: now,
+            stake_data_after_unstaking_error: None,
         })
     }
 }
@@ -60,7 +64,7 @@ impl<BlockNumber> ProposalStatus<BlockNumber> {
 /// Final proposal status and potential error.
 #[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
 #[derive(Encode, Decode, Clone, PartialEq, Eq, Debug)]
-pub struct FinalizationData<BlockNumber> {
+pub struct FinalizationData<BlockNumber, StakeId, AccountId> {
     /// Final proposal status
     pub proposal_status: ProposalDecisionStatus,
 
@@ -69,14 +73,17 @@ pub struct FinalizationData<BlockNumber> {
 
     /// Error occured during the proposal finalization - unstaking failed in the stake module
     pub encoded_unstaking_error_due_to_broken_runtime: Option<Vec<u8>>,
+
+    /// Stake data for the proposal, filled if the unstaking wasn't successful
+    pub stake_data_after_unstaking_error: Option<ActiveStake<StakeId, AccountId>>,
 }
 
-impl<BlockNumber> FinalizationData<BlockNumber> {
+impl<BlockNumber, StakeId, AccountId> FinalizationData<BlockNumber, StakeId, AccountId> {
     /// FinalizationData helper, creates ApprovedProposalStatus
     pub fn create_approved_proposal_status(
         self,
         approved_status: ApprovedProposalStatus,
-    ) -> ProposalStatus<BlockNumber> {
+    ) -> ProposalStatus<BlockNumber, StakeId, AccountId> {
         ProposalStatus::Finalized(FinalizationData {
             proposal_status: ProposalDecisionStatus::Approved(approved_status),
             ..self