Migrate all proposals to the new serialization model

- change proposals codex extrinscis
- enrich runtime proposal encoder
- restore tests

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

@@ -43,14 +43,13 @@
 
 mod proposal_types;
 
-// #[cfg(test)]
-// mod tests;
+#[cfg(test)]
+mod tests;
 
-use codec::Encode;
 use common::origin_validator::ActorOriginValidator;
 use governance::election_params::ElectionParameters;
 use proposal_engine::ProposalParameters;
-use roles::actors::{Role, RoleParameters};
+use roles::actors::RoleParameters;
 use rstd::clone::Clone;
 use rstd::convert::TryInto;
 use rstd::prelude::*;
@@ -65,24 +64,12 @@ use srml_support::traits::{Currency, Get};
 use srml_support::{decl_error, decl_module, decl_storage, ensure, print};
 use system::{ensure_root, RawOrigin};
 
-pub use proposal_types::ProposalDetails;
+pub use proposal_types::{ProposalDetails, ProposalDetailsOf, ProposalEncoder};
 
 // Percentage of the total token issue as max mint balance value. Shared with spending
 // proposal max balance percentage.
 const COUNCIL_MINT_MAX_BALANCE_PERCENT: u32 = 2;
 
-pub trait ProposalEncoder<T: Trait> {
-    fn encode_proposal(
-        proposal_details: ProposalDetails<
-            BalanceOfMint<T>,
-            BalanceOfGovernanceCurrency<T>,
-            T::BlockNumber,
-            T::AccountId,
-            MemberId<T>,
-        >,
-    ) -> Vec<u8>;
-}
-
 /// 'Proposals codex' substrate module Trait
 pub trait Trait:
     system::Trait
@@ -370,7 +357,7 @@ decl_module! {
                 proposal_details,
             )?;
         }
-/*
+
         /// Create 'Runtime upgrade' proposal type. Runtime upgrade can be initiated only by
         /// members from the hardcoded list `RuntimeUpgradeProposalAllowedProposers`
         pub fn create_runtime_upgrade_proposal(
@@ -387,20 +374,23 @@ decl_module! {
 
             let wasm_hash = blake2_256(&wasm);
 
-            let proposal_code =
-                <Call<T>>::execute_runtime_upgrade_proposal(title.clone(), description.clone(), wasm);
+            // The runtime upgrade proposal has two proposal details: wasm and wasm hash.
+            // This is an exception for the optimization.
 
             let proposal_parameters = proposal_types::parameters::runtime_upgrade_proposal::<T>();
+            let proposal_details = ProposalDetails::RuntimeUpgrade(wasm);
+            let proposal_code = T::ProposalEncoder::encode_proposal(proposal_details.clone());
 
+            let proposal_details_for_hash = ProposalDetails::RuntimeUpgradeHash(wasm_hash.to_vec());
             Self::create_proposal(
                 origin,
                 member_id,
                 title,
                 description,
                 stake_balance,
-                proposal_code.encode(),
+                proposal_code,
                 proposal_parameters,
-                ProposalDetails::RuntimeUpgrade(wasm_hash.to_vec()),
+                proposal_details_for_hash,
             )?;
         }
 
@@ -418,9 +408,8 @@ decl_module! {
 
             Self::ensure_council_election_parameters_valid(&election_parameters)?;
 
-            let proposal_code =
-                <governance::election::Call<T>>::set_election_parameters(election_parameters.clone());
-
+            let proposal_details = ProposalDetails::SetElectionParameters(election_parameters);
+            let proposal_code = T::ProposalEncoder::encode_proposal(proposal_details.clone());
             let proposal_parameters =
                 proposal_types::parameters::set_election_parameters_proposal::<T>();
 
@@ -430,9 +419,9 @@ decl_module! {
                 title,
                 description,
                 stake_balance,
-                proposal_code.encode(),
+                proposal_code,
                 proposal_parameters,
-                ProposalDetails::SetElectionParameters(election_parameters),
+                proposal_details,
             )?;
         }
 
@@ -455,11 +444,10 @@ decl_module! {
                 Error::InvalidStorageWorkingGroupMintCapacity
             );
 
-            let proposal_code =
-                <content_working_group::Call<T>>::set_mint_capacity(mint_balance.clone());
-
             let proposal_parameters =
                 proposal_types::parameters::set_content_working_group_mint_capacity_proposal::<T>();
+            let proposal_details = ProposalDetails::SetContentWorkingGroupMintCapacity(mint_balance);
+            let proposal_code = T::ProposalEncoder::encode_proposal(proposal_details.clone());
 
             Self::create_proposal(
                 origin,
@@ -467,9 +455,9 @@ decl_module! {
                 title,
                 description,
                 stake_balance,
-                proposal_code.encode(),
+                proposal_code,
                 proposal_parameters,
-                ProposalDetails::SetContentWorkingGroupMintCapacity(mint_balance),
+                proposal_details,
             )?;
         }
 
@@ -498,13 +486,10 @@ decl_module! {
                 Error::InvalidSpendingProposalBalance
             );
 
-            let proposal_code = <governance::council::Call<T>>::spend_from_council_mint(
-                balance.clone(),
-                destination.clone()
-            );
-
             let proposal_parameters =
                 proposal_types::parameters::spending_proposal::<T>();
+            let proposal_details = ProposalDetails::Spending(balance, destination);
+            let proposal_code = T::ProposalEncoder::encode_proposal(proposal_details.clone());
 
             Self::create_proposal(
                 origin,
@@ -512,13 +497,12 @@ decl_module! {
                 title,
                 description,
                 stake_balance,
-                proposal_code.encode(),
+                proposal_code,
                 proposal_parameters,
-                ProposalDetails::Spending(balance, destination),
+                proposal_details,
             )?;
         }
 
-
         /// Create 'Set lead' proposal type.
         /// This proposal uses `replace_lead()` extrinsic from the `content_working_group`  module.
         pub fn create_set_lead_proposal(
@@ -537,11 +521,10 @@ decl_module! {
                 );
             }
 
-            let proposal_code =
-                <content_working_group::Call<T>>::replace_lead(new_lead.clone());
-
             let proposal_parameters =
                 proposal_types::parameters::set_lead_proposal::<T>();
+            let proposal_details = ProposalDetails::SetLead(new_lead);
+            let proposal_code = T::ProposalEncoder::encode_proposal(proposal_details.clone());
 
             Self::create_proposal(
                 origin,
@@ -549,9 +532,9 @@ decl_module! {
                 title,
                 description,
                 stake_balance,
-                proposal_code.encode(),
+                proposal_code,
                 proposal_parameters,
-                ProposalDetails::SetLead(new_lead),
+                proposal_details,
             )?;
         }
 
@@ -565,11 +548,10 @@ decl_module! {
             stake_balance: Option<BalanceOf<T>>,
             actor_account: T::AccountId,
         ) {
-            let proposal_code =
-                <roles::actors::Call<T>>::remove_actor(actor_account.clone());
-
             let proposal_parameters =
                 proposal_types::parameters::evict_storage_provider_proposal::<T>();
+            let proposal_details = ProposalDetails::EvictStorageProvider(actor_account);
+            let proposal_code = T::ProposalEncoder::encode_proposal(proposal_details.clone());
 
             Self::create_proposal(
                 origin,
@@ -577,9 +559,9 @@ decl_module! {
                 title,
                 description,
                 stake_balance,
-                proposal_code.encode(),
+                proposal_code,
                 proposal_parameters,
-                ProposalDetails::EvictStorageProvider(actor_account),
+                proposal_details,
             )?;
         }
 
@@ -603,11 +585,10 @@ decl_module! {
                 Error::InvalidValidatorCount
             );
 
-            let proposal_code =
-                <staking::Call<T>>::set_validator_count(new_validator_count);
-
             let proposal_parameters =
                 proposal_types::parameters::set_validator_count_proposal::<T>();
+            let proposal_details = ProposalDetails::SetValidatorCount(new_validator_count);
+            let proposal_code = T::ProposalEncoder::encode_proposal(proposal_details.clone());
 
             Self::create_proposal(
                 origin,
@@ -615,9 +596,9 @@ decl_module! {
                 title,
                 description,
                 stake_balance,
-                proposal_code.encode(),
+                proposal_code,
                 proposal_parameters,
-                ProposalDetails::SetValidatorCount(new_validator_count),
+                proposal_details,
             )?;
         }
 
@@ -633,13 +614,10 @@ decl_module! {
         ) {
             Self::ensure_storage_role_parameters_valid(&role_parameters)?;
 
-            let proposal_code = <roles::actors::Call<T>>::set_role_parameters(
-                Role::StorageProvider,
-                role_parameters.clone()
-            );
-
             let proposal_parameters =
                 proposal_types::parameters::set_storage_role_parameters_proposal::<T>();
+            let proposal_details =  ProposalDetails::SetStorageRoleParameters(role_parameters);
+            let proposal_code = T::ProposalEncoder::encode_proposal(proposal_details.clone());
 
             Self::create_proposal(
                 origin,
@@ -647,12 +625,12 @@ decl_module! {
                 title,
                 description,
                 stake_balance,
-                proposal_code.encode(),
+                proposal_code,
                 proposal_parameters,
-                ProposalDetails::SetStorageRoleParameters(role_parameters),
+                proposal_details,
             )?;
         }
-*/
+
 // *************** Extrinsic to execute
 
         /// Text proposal extrinsic. Should be used as callable object to pass to the `engine` module.
@@ -672,20 +650,16 @@ decl_module! {
         /// Should be used as callable object to pass to the `engine` module.
         pub fn execute_runtime_upgrade_proposal(
             origin,
-            title: Vec<u8>,
-            _description: Vec<u8>,
             wasm: Vec<u8>,
         ) {
             let (cloned_origin1, cloned_origin2) =  Self::double_origin(origin);
             ensure_root(cloned_origin1)?;
 
-            print("Runtime upgrade proposal: ");
-            let title_string_result = from_utf8(title.as_slice());
-            if let Ok(title_string) = title_string_result{
-                print(title_string);
-            }
+            print("Runtime upgrade proposal execution started.");
 
             <system::Module<T>>::set_code(cloned_origin2, wasm)?;
+
+            print("Runtime upgrade proposal execution finished.");
         }
     }
 }

runtime-modules/proposals/codex/src/proposal_types/mod.rs

@@ -8,6 +8,21 @@ use serde::{Deserialize, Serialize};
 use crate::ElectionParameters;
 use roles::actors::RoleParameters;
 
+/// Encodes proposal using its details information.
+pub trait ProposalEncoder<T: crate::Trait> {
+    /// Encodes proposal using its details information.
+    fn encode_proposal(proposal_details: ProposalDetailsOf<T>) -> Vec<u8>;
+}
+
+/// _ProposalDetails_ alias for type simplification
+pub type ProposalDetailsOf<T> = ProposalDetails<
+    crate::BalanceOfMint<T>,
+    crate::BalanceOfGovernanceCurrency<T>,
+    <T as system::Trait>::BlockNumber,
+    <T as system::Trait>::AccountId,
+    crate::MemberId<T>,
+>;
+
 /// Proposal details provide voters the information required for the perceived voting.
 #[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
 #[derive(Encode, Decode, Clone, PartialEq, Debug)]
@@ -15,7 +30,12 @@ pub enum ProposalDetails<MintedBalance, CurrencyBalance, BlockNumber, AccountId,
     /// The text of the `text` proposal
     Text(Vec<u8>),
 
-    /// The hash of wasm code for the `runtime upgrade` proposal
+    /// The hash of wasm code for the `runtime upgrade` proposal. The runtime upgrade proposal has
+    /// two proposal details: wasm and wasm hash. This is an exception for the optimization.
+    RuntimeUpgradeHash(Vec<u8>),
+
+    /// The wasm code for the `runtime upgrade` proposal. The runtime upgrade proposal has
+    /// two proposal details: wasm and wasm hash. This is an exception for the optimization.
     RuntimeUpgrade(Vec<u8>),
 
     /// Election parameters for the `set election parameters` proposal

runtime-modules/proposals/codex/src/proposal_types/parameters.rs

@@ -126,33 +126,32 @@ pub(crate) fn set_storage_role_parameters_proposal<T: crate::Trait>(
     }
 }
 
-//TODO: uncomment
-// #[cfg(test)]
-// mod test {
-//     use crate::proposal_types::parameters::get_required_stake_by_fraction;
-//     use crate::tests::{increase_total_balance_issuance, initial_test_ext, Test};
-//
-//     pub use sr_primitives::Perbill;
-//
-//     #[test]
-//     fn calculate_get_required_stake_by_fraction_with_zero_issuance() {
-//         initial_test_ext()
-//             .execute_with(|| assert_eq!(get_required_stake_by_fraction::<Test>(5, 7), 0));
-//     }
-//
-//     #[test]
-//     fn calculate_stake_by_percentage_for_defined_issuance_succeeds() {
-//         initial_test_ext().execute_with(|| {
-//             increase_total_balance_issuance(50000);
-//             assert_eq!(get_required_stake_by_fraction::<Test>(1, 1000), 50)
-//         });
-//     }
-//
-//     #[test]
-//     fn calculate_stake_by_percentage_for_defined_issuance_with_fraction_loss() {
-//         initial_test_ext().execute_with(|| {
-//             increase_total_balance_issuance(1111);
-//             assert_eq!(get_required_stake_by_fraction::<Test>(3, 1000), 3);
-//         });
-//     }
-// }
+#[cfg(test)]
+mod test {
+    use crate::proposal_types::parameters::get_required_stake_by_fraction;
+    use crate::tests::{increase_total_balance_issuance, initial_test_ext, Test};
+
+    pub use sr_primitives::Perbill;
+
+    #[test]
+    fn calculate_get_required_stake_by_fraction_with_zero_issuance() {
+        initial_test_ext()
+            .execute_with(|| assert_eq!(get_required_stake_by_fraction::<Test>(5, 7), 0));
+    }
+
+    #[test]
+    fn calculate_stake_by_percentage_for_defined_issuance_succeeds() {
+        initial_test_ext().execute_with(|| {
+            increase_total_balance_issuance(50000);
+            assert_eq!(get_required_stake_by_fraction::<Test>(1, 1000), 50)
+        });
+    }
+
+    #[test]
+    fn calculate_stake_by_percentage_for_defined_issuance_with_fraction_loss() {
+        initial_test_ext().execute_with(|| {
+            increase_total_balance_issuance(1111);
+            assert_eq!(get_required_stake_by_fraction::<Test>(3, 1000), 3);
+        });
+    }
+}

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

@@ -3,6 +3,7 @@
 // TODO: remove after post-Rome substrate upgrade
 #![allow(array_into_iter)]
 
+use crate::{ProposalDetailsOf, ProposalEncoder};
 pub use primitives::{Blake2Hasher, H256};
 use proposal_engine::VotersParameters;
 use sr_primitives::curve::PiecewiseLinear;
@@ -249,6 +250,13 @@ impl crate::Trait for Test {
     type TextProposalMaxLength = TextProposalMaxLength;
     type RuntimeUpgradeWasmProposalMaxLength = RuntimeUpgradeWasmProposalMaxLength;
     type MembershipOriginValidator = ();
+    type ProposalEncoder = ();
+}
+
+impl ProposalEncoder<Test> for () {
+    fn encode_proposal(_proposal_details: ProposalDetailsOf<Test>) -> Vec<u8> {
+        Vec::new()
+    }
 }
 
 impl system::Trait for Test {

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

@@ -223,7 +223,7 @@ fn create_runtime_upgrade_common_checks_succeed() {
                 )
             },
             proposal_parameters: crate::proposal_types::parameters::runtime_upgrade_proposal::<Test>(),
-            proposal_details: ProposalDetails::RuntimeUpgrade(blake2_256(b"wasm").to_vec()),
+            proposal_details: ProposalDetails::RuntimeUpgradeHash(blake2_256(b"wasm").to_vec()),
         };
         proposal_fixture.check_all();
     });

runtime/src/integration/proposals/proposal_encoder.rs

@@ -1,25 +1,67 @@
-use crate::integration::proposals::MemberId;
-use crate::*;
-use proposals_codex::{ProposalDetails, ProposalEncoder};
+use crate::{Call, Runtime};
+use proposals_codex::{ProposalDetails, ProposalDetailsOf, ProposalEncoder};
+use roles::actors::Role;
 
-pub struct ExtrinsicProposalEncoder;
+use codec::Encode;
+use rstd::vec::Vec;
+use srml_support::print;
 
+/// _ProposalEncoder_ implementation. It encodes extrinsics with proposal details parameters
+/// using Runtime Call and parity codec.
+pub struct ExtrinsicProposalEncoder;
 impl ProposalEncoder<Runtime> for ExtrinsicProposalEncoder {
-    fn encode_proposal(
-        proposal_details: ProposalDetails<
-            Balance,
-            Balance,
-            BlockNumber,
-            AccountId,
-            MemberId<Runtime>,
-        >,
-    ) -> Vec<u8> {
+    fn encode_proposal(proposal_details: ProposalDetailsOf<Runtime>) -> Vec<u8> {
         match proposal_details {
             ProposalDetails::Text(text) => {
-                crate::Call::ProposalsCodex(proposals_codex::Call::execute_text_proposal(text))
+                Call::ProposalsCodex(proposals_codex::Call::execute_text_proposal(text)).encode()
+            }
+            ProposalDetails::SetElectionParameters(election_parameters) => Call::CouncilElection(
+                governance::election::Call::set_election_parameters(election_parameters),
+            )
+            .encode(),
+            ProposalDetails::SetContentWorkingGroupMintCapacity(mint_balance) => {
+                Call::ContentWorkingGroup(content_working_group::Call::set_mint_capacity(
+                    mint_balance,
+                ))
+                .encode()
+            }
+            ProposalDetails::Spending(balance, destination) => Call::Council(
+                governance::council::Call::spend_from_council_mint(balance, destination),
+            )
+            .encode(),
+            ProposalDetails::SetLead(new_lead) => {
+                Call::ContentWorkingGroup(content_working_group::Call::replace_lead(new_lead))
                     .encode()
             }
-            _ => unreachable!(),
+            ProposalDetails::EvictStorageProvider(actor_account) => {
+                Call::Actors(roles::actors::Call::remove_actor(actor_account)).encode()
+            }
+            ProposalDetails::SetValidatorCount(new_validator_count) => {
+                Call::Staking(staking::Call::set_validator_count(new_validator_count)).encode()
+            }
+            ProposalDetails::SetStorageRoleParameters(role_parameters) => Call::Actors(
+                roles::actors::Call::set_role_parameters(Role::StorageProvider, role_parameters),
+            )
+            .encode(),
+            ProposalDetails::RuntimeUpgrade(wasm_code) => {
+                // The runtime upgrade proposal has two proposal details: wasm and wasm hash.
+                // This is an exception for the optimization.
+
+                // This is a real extrinsic call.
+                Call::ProposalsCodex(proposals_codex::Call::execute_runtime_upgrade_proposal(
+                    wasm_code,
+                ))
+                .encode()
+            }
+            ProposalDetails::RuntimeUpgradeHash(_) => {
+                // The runtime upgrade proposal has two proposal details: wasm and wasm hash.
+                // This is an exception for the optimization.
+
+                // Cannot be here. This is a bug.
+                print("Invalid proposal: cannot encode ProposalDetails::RuntimeUpgradeHash");
+
+                Vec::new()
+            }
         }
     }
 }

runtime/src/test/proposals_integration.rs

@@ -396,8 +396,6 @@ fn text_proposal_execution_succeeds() {
         setup_members(7);
         setup_council();
 
-        println!("{}", CouncilManager::<Runtime>::total_voters_count());
-
         let member_id = 1;
         let account_id: [u8; 32] = [member_id; 32];
         increase_total_balance_issuance_using_account_id(account_id.clone().into(), 500000);