Proposals v2 iteration2

modules/proposals/codex/src/tests/mock.rs

@@ -45,6 +45,12 @@ impl proposal_engine::Trait for Test {
     type TotalVotersCounter = MockVotersParameters;
 
     type ProposalCodeDecoder = crate::ProposalType;
+
+    type ProposalId = u32;
+
+    type ProposerId = u64;
+
+    type VoterId = u64;
 }
 
 pub struct MockVotersParameters;

modules/proposals/engine/src/lib.rs

@@ -16,6 +16,7 @@
 // Do not delete! Cannot be uncommented by default, because of Parity decl_module! issue.
 //#![warn(missing_docs)]
 
+use types::FinalizedProposalData;
 pub use types::TallyResult;
 pub use types::{Proposal, ProposalParameters, ProposalStatus};
 pub use types::{ProposalCodeDecoder, ProposalExecutable};
@@ -27,13 +28,18 @@ mod types;
 #[cfg(test)]
 mod tests;
 
-use rstd::collections::btree_set::BTreeSet;
 use rstd::prelude::*;
 use runtime_primitives::traits::EnsureOrigin;
-use srml_support::{decl_event, decl_module, decl_storage, dispatch, ensure, StorageDoubleMap};
+use srml_support::{
+    decl_event, decl_module, decl_storage, dispatch, ensure, Parameter, StorageDoubleMap,
+};
 use system::ensure_root;
 
+// TODO: add maximum allowed active proposals
+
+// Max allowed proposal title length. Can be used if config value is not filled.
 const DEFAULT_TITLE_MAX_LEN: u32 = 100;
+// Max allowed proposal body length. Can be used if config value is not filled.
 const DEFAULT_BODY_MAX_LEN: u32 = 10_000;
 
 /// Proposals engine trait.
@@ -52,70 +58,75 @@ pub trait Trait: system::Trait + timestamp::Trait {
 
     /// Converts proposal code binary to executable representation
     type ProposalCodeDecoder: ProposalCodeDecoder;
+
+    /// Proposal Id type
+    type ProposalId: From<u32> + Parameter + Default + Copy;
+
+    /// Type for the proposer id. Should be authenticated by account id.
+    type ProposerId: From<Self::AccountId> + Parameter + Default;
+
+    /// Type for the voter id. Should be authenticated by account id.
+    type VoterId: From<Self::AccountId> + Parameter + Default + Clone;
 }
 
 decl_event!(
     pub enum Event<T>
     where
-        <T as system::Trait>::AccountId
+        <T as Trait>::ProposalId,
+        <T as Trait>::ProposerId,
+        <T as Trait>::VoterId,
     {
     	/// Emits on proposal creation.
         /// Params:
         /// * Account id of a proposer.
         /// * Id of a newly created proposal after it was saved in storage.
-        ProposalCreated(AccountId, u32),
+        ProposalCreated(ProposerId, ProposalId),
 
         /// Emits on proposal cancellation.
         /// Params:
         /// * Account id of a proposer.
         /// * Id of a cancelled proposal.
-        ProposalCanceled(AccountId, u32),
+        ProposalCanceled(ProposerId, ProposalId),
 
         /// Emits on proposal veto.
         /// Params:
         /// * Id of a vetoed proposal.
-        ProposalVetoed(u32),
+        ProposalVetoed(ProposalId),
 
         /// Emits on proposal status change.
         /// Params:
         /// * Id of a updated proposal.
         /// * New proposal status
-        ProposalStatusUpdated(u32, ProposalStatus),
+        ProposalStatusUpdated(ProposalId, ProposalStatus),
 
         /// Emits on voting for the proposal
         /// Params:
         /// * Voter - an account id of a voter.
         /// * Id of a proposal.
         /// * Kind of vote.
-        Voted(AccountId, u32, VoteKind),
+        Voted(VoterId, ProposalId, VoteKind),
     }
 );
 
 // Storage for the proposals module
 decl_storage! {
     trait Store for Module<T: Trait> as ProposalsEngine{
         /// Map proposal by its id.
-        pub Proposals get(fn proposals): map u32 => Proposal<T::BlockNumber, T::AccountId>;
+        pub Proposals get(fn proposals): map T::ProposalId =>
+            Proposal<T::BlockNumber, T::VoterId, T::ProposerId>;
 
         /// Count of all proposals that have been created.
         pub ProposalCount get(fn proposal_count): u32;
 
         /// Map proposal executable code by proposal id.
-        ProposalCode get(fn proposal_codes): map u32 =>  Vec<u8>;
-
-        /// Map votes by proposal id.
-        VotesByProposalId get(fn votes_by_proposal): map u32 => Vec<Vote<T::AccountId>>;
+        ProposalCode get(fn proposal_codes): map T::ProposalId =>  Vec<u8>;
 
         /// Ids of proposals that are open for voting (have not been finalized yet).
-        pub ActiveProposalIds get(fn active_proposal_ids): BTreeSet<u32>;
-
-        /// Proposal tally results map
-        pub(crate) TallyResults get(fn tally_results): map u32 => TallyResult<T::BlockNumber>;
-
-        /// Double map for preventing duplicate votes
-        VoteExistsByAccountByProposal get(fn vote_by_proposal_by_account):
-            double_map T::AccountId, twox_256(u32) => ();
+        pub ActiveProposalIds get(fn active_proposal_ids): linked_map T::ProposalId => ();
 
+        /// Double map for preventing duplicate votes. Should be cleaned after usage.
+        pub(crate) VoteExistsByProposalByVoter get(fn vote_by_proposal_by_voter):
+            double_map T::ProposalId, twox_256(T::VoterId) => ();
 
         /// Defines max allowed proposal title length. Can be configured.
         TitleMaxLen get(title_max_len) config(): u32 = DEFAULT_TITLE_MAX_LEN;
@@ -133,20 +144,21 @@ decl_module! {
         fn deposit_event() = default;
 
         /// Vote extrinsic. Conditions:  origin must allow votes.
-        pub fn vote(origin, proposal_id: u32, vote: VoteKind)  {
-            let voter_id = T::VoteOrigin::ensure_origin(origin)?;
+        pub fn vote(origin, proposal_id: T::ProposalId, vote: VoteKind)  {
+            let account_id = T::VoteOrigin::ensure_origin(origin)?;
+            let voter_id = T::VoterId::from(account_id);
 
             ensure!(<Proposals<T>>::exists(proposal_id), errors::MSG_PROPOSAL_NOT_FOUND);
-            let proposal = Self::proposals(proposal_id);
+            let mut proposal = Self::proposals(proposal_id);
 
             let not_expired = !proposal.is_voting_period_expired(Self::current_block());
             ensure!(not_expired, errors::MSG_PROPOSAL_EXPIRED);
 
             ensure!(proposal.status == ProposalStatus::Active, errors::MSG_PROPOSAL_FINALIZED);
 
-            let did_not_vote_before = !<VoteExistsByAccountByProposal<T>>::exists(
+            let did_not_vote_before = !<VoteExistsByProposalByVoter<T>>::exists(
+                proposal_id,
                 voter_id.clone(),
-                proposal_id
             );
 
             ensure!(did_not_vote_before, errors::MSG_YOU_ALREADY_VOTED);
@@ -156,16 +168,19 @@ decl_module! {
                 vote_kind: vote.clone(),
             };
 
+            proposal.votes.push(new_vote);
+
             // mutation
 
-            <VotesByProposalId<T>>::mutate(proposal_id, |votes| votes.push(new_vote));
-            <VoteExistsByAccountByProposal<T>>::insert(voter_id.clone(), proposal_id, ());
+            <Proposals<T>>::insert(proposal_id, proposal);
+            <VoteExistsByProposalByVoter<T>>::insert( proposal_id, voter_id.clone(), ());
             Self::deposit_event(RawEvent::Voted(voter_id, proposal_id, vote));
         }
 
         /// Cancel a proposal by its original proposer.
-        pub fn cancel_proposal(origin, proposal_id: u32) {
-            let proposer_id = T::ProposalOrigin::ensure_origin(origin)?;
+        pub fn cancel_proposal(origin, proposal_id: T::ProposalId) {
+            let account_id = T::ProposalOrigin::ensure_origin(origin)?;
+            let proposer_id = T::ProposerId::from(account_id);
 
             ensure!(<Proposals<T>>::exists(proposal_id), errors::MSG_PROPOSAL_NOT_FOUND);
             let proposal = Self::proposals(proposal_id);
@@ -180,7 +195,7 @@ decl_module! {
         }
 
         /// Veto a proposal. Must be root.
-        pub fn veto_proposal(origin, proposal_id: u32) {
+        pub fn veto_proposal(origin, proposal_id: T::ProposalId) {
             ensure_root(origin)?;
 
             ensure!(<Proposals<T>>::exists(proposal_id), errors::MSG_PROPOSAL_NOT_FOUND);
@@ -196,14 +211,12 @@ decl_module! {
 
         /// Block finalization. Perform voting period check and vote result tally.
         fn on_finalize(_n: T::BlockNumber) {
-            let tally_results = Self::tally();
+            let finalized_proposals_data = Self::get_finalized_proposals_data();
 
             // mutation
-
-            for  tally_result in tally_results {
-                <TallyResults<T>>::insert(tally_result.proposal_id, &tally_result);
-
-                Self::update_proposal_status(tally_result.proposal_id, tally_result.status);
+            for  proposal_data in finalized_proposals_data {
+                <Proposals<T>>::insert(proposal_data.proposal_id, proposal_data.proposal);
+                Self::update_proposal_status(proposal_data.proposal_id, proposal_data.status);
             }
         }
     }
@@ -219,7 +232,8 @@ impl<T: Trait> Module<T> {
         proposal_type: u32,
         proposal_code: Vec<u8>,
     ) -> dispatch::Result {
-        let proposer_id = T::ProposalOrigin::ensure_origin(origin)?;
+        let account_id = T::ProposalOrigin::ensure_origin(origin)?;
+        let proposer_id = T::ProposerId::from(account_id);
 
         ensure!(!title.is_empty(), errors::MSG_EMPTY_TITLE_PROVIDED);
         ensure!(
@@ -244,15 +258,18 @@ impl<T: Trait> Module<T> {
             proposer_id: proposer_id.clone(),
             proposal_type,
             status: ProposalStatus::Active,
+            tally_results: None,
+            votes: Vec::new(),
         };
 
         // mutation
-        <Proposals<T>>::insert(new_proposal_id, new_proposal);
-        <ProposalCode>::insert(new_proposal_id, proposal_code);
-        ActiveProposalIds::mutate(|ids| ids.insert(new_proposal_id));
+        let proposal_id = T::ProposalId::from(new_proposal_id);
+        <Proposals<T>>::insert(proposal_id, new_proposal);
+        <ProposalCode<T>>::insert(proposal_id, proposal_code);
+        <ActiveProposalIds<T>>::insert(proposal_id, ());
         ProposalCount::put(next_proposal_count_value);
 
-        Self::deposit_event(RawEvent::ProposalCreated(proposer_id, new_proposal_id));
+        Self::deposit_event(RawEvent::ProposalCreated(proposer_id, proposal_id));
 
         Ok(())
     }
@@ -265,7 +282,7 @@ impl<T: Trait> Module<T> {
     }
 
     // Executes approved proposal code
-    fn execute_proposal(proposal_id: u32) {
+    fn execute_proposal(proposal_id: T::ProposalId) {
         //let origin = system::RawOrigin::Root.into();
         let proposal = Self::proposals(proposal_id);
         let proposal_code = Self::proposal_codes(proposal_id);
@@ -291,31 +308,54 @@ impl<T: Trait> Module<T> {
         Self::update_proposal_status(proposal_id, new_proposal_status)
     }
 
-    /// Voting results tally.
-    /// Returns proposals with changed status and tally results
-    fn tally() -> Vec<TallyResult<T::BlockNumber>> {
-        let mut results = Vec::new();
-        for &proposal_id in Self::active_proposal_ids().iter() {
-            let votes = Self::votes_by_proposal(proposal_id);
-            let proposal = Self::proposals(proposal_id);
-
-            if let Some(tally_result) = proposal.tally_results(
-                proposal_id,
-                votes,
-                T::TotalVotersCounter::total_voters_count(),
-                Self::current_block(),
-            ) {
-                results.push(tally_result);
-            }
-        }
-
-        results
+    /// Enumerates through active proposals. Tally Voting results.
+    /// Returns proposals with changed status, id and calculated tally results
+    fn get_finalized_proposals_data(
+    ) -> Vec<FinalizedProposalData<T::ProposalId, T::BlockNumber, T::VoterId, T::ProposerId>>
+    {
+        // enumerate active proposals id and gather finalization data
+        <ActiveProposalIds<T>>::enumerate()
+            .map(|(proposal_id, _)| {
+                // load current proposal
+                let mut proposal = Self::proposals(proposal_id);
+
+                // calculates voting results
+                proposal.update_tally_results(
+                    T::TotalVotersCounter::total_voters_count(),
+                    Self::current_block(),
+                );
+
+                // get new status from tally results
+                let mut new_status = ProposalStatus::Active;
+                if let Some(tally_results) = proposal.tally_results.clone() {
+                    new_status = tally_results.status;
+                }
+                // proposal is finalized if not active
+                let finalized = new_status != ProposalStatus::Active;
+
+                (
+                    FinalizedProposalData {
+                        proposal_id,
+                        proposal,
+                        status: new_status,
+                    },
+                    finalized,
+                )
+            })
+            .filter(|(_, finalized)| *finalized) // filter only finalized proposals
+            .map(|(data, _)| data) // get rid of used 'finalized' flag
+            .collect() // compose output vector
     }
 
+    // TODO: to be refactored or removed after introducing stakes. Events should be fired on actions
+    // such as 'rejected' or 'approved'.
     /// Updates proposal status and removes proposal id from active id set.
-    fn update_proposal_status(proposal_id: u32, new_status: ProposalStatus) {
+    fn update_proposal_status(proposal_id: T::ProposalId, new_status: ProposalStatus) {
+        if new_status != ProposalStatus::Active {
+            <ActiveProposalIds<T>>::remove(&proposal_id);
+            <VoteExistsByProposalByVoter<T>>::remove_prefix(&proposal_id);
+        }
         <Proposals<T>>::mutate(proposal_id, |p| p.status = new_status.clone());
-        ActiveProposalIds::mutate(|ids| ids.remove(&proposal_id));
 
         Self::deposit_event(RawEvent::ProposalStatusUpdated(
             proposal_id,
@@ -327,22 +367,15 @@ impl<T: Trait> Module<T> {
                 Self::reject_proposal(proposal_id)
             }
             ProposalStatus::Approved => Self::approve_proposal(proposal_id),
-            ProposalStatus::Active => {
-                // restore active proposal id
-                ActiveProposalIds::mutate(|ids| ids.insert(proposal_id));
-            }
-            ProposalStatus::Executed
-            | ProposalStatus::Failed { .. }
-            | ProposalStatus::Vetoed
-            | ProposalStatus::Canceled => {} // do nothing
+            _ => {} // do nothing
         }
     }
 
     /// Reject a proposal. The staked deposit will be returned to a proposer.
-    fn reject_proposal(_proposal_id: u32) {}
+    fn reject_proposal(_proposal_id: T::ProposalId) {}
 
     /// Approve a proposal. The staked deposit will be returned.
-    fn approve_proposal(proposal_id: u32) {
+    fn approve_proposal(proposal_id: T::ProposalId) {
         Self::execute_proposal(proposal_id);
     }
 }

modules/proposals/engine/src/tests/mock.rs

@@ -54,6 +54,12 @@ impl crate::Trait for Test {
     type TotalVotersCounter = ();
 
     type ProposalCodeDecoder = ProposalType;
+
+    type ProposalId = u32;
+
+    type ProposerId = u64;
+
+    type VoterId = u64;
 }
 
 impl VotersParameters for () {