Merge pull request #3275 from mnaamani/fix-compile-time-proposal-params-config

Olympia: Improved handling of proposal params through json config

build-node-docker.sh

@@ -8,6 +8,8 @@ set -e
 CODE_HASH=`scripts/runtime-code-shasum.sh`
 IMAGE=joystream/node:${CODE_HASH}
 
+# TODO: Check for valid JSON in ALL_PROPOSALS_PARAMETERS_JSON ?
+
 # Look for image locally
 if ! docker inspect ${IMAGE} > /dev/null;
 then
@@ -19,7 +21,9 @@ then
   if ! docker inspect ${IMAGE} > /dev/null;
   then
     echo "Building ${IMAGE}.."
-    docker build . --file joystream-node.Dockerfile --tag ${IMAGE} --build-arg TEST_NODE=${TEST_NODE}
+    docker build . --file joystream-node.Dockerfile --tag ${IMAGE} \
+	    --build-arg TEST_NODE=${TEST_NODE} \
+	    --build-arg ALL_PROPOSALS_PARAMETERS_JSON=${ALL_PROPOSALS_PARAMETERS_JSON}
   fi
 else
   echo "Found ${IMAGE} in local repo"

devops/aws/roles/common/tasks/run-setup-build.yml

@@ -13,8 +13,8 @@
   shell: . ~/.bash_profile && yarn cargo-build
   args:
     chdir: '{{ remote_code_path }}'
-  # environment:
-  #   ALL_PROPOSALS_PARAMETERS_JSON: '{{ proposal_parameters }}'
+  environment:
+    ALL_PROPOSALS_PARAMETERS_JSON: '{{ proposal_parameters }}'
   # Run in async fashion for max duration of 1 hr
   async: 3600
   poll: 0

joystream-node.Dockerfile

@@ -14,13 +14,14 @@ COPY . /joystream
 
 # Build all cargo crates
 # Ensure our tests and linter pass before actual build
-ENV WASM_BUILD_TOOLCHAIN=nightly-2021-02-20
+ARG ALL_PROPOSALS_PARAMETERS_JSON
 ARG TEST_NODE
 RUN echo "TEST_NODE=$TEST_NODE"
 RUN test -n "$TEST_NODE" && sed -i 's/MILLISECS_PER_BLOCK: Moment = 6000/MILLISECS_PER_BLOCK: Moment = 1000/' ./runtime/src/constants.rs; exit 0
 RUN test -n "$TEST_NODE" && sed -i 's/SLOT_DURATION: Moment = 6000/SLOT_DURATION: Moment = 1000/' ./runtime/src/constants.rs; exit 0
 RUN test -n "$TEST_NODE" && export ALL_PROPOSALS_PARAMETERS_JSON="$(cat ./tests/integration-tests/proposal-parameters.json)";\
     echo "ALL_PROPOSALS_PARAMETERS_JSON=$ALL_PROPOSALS_PARAMETERS_JSON" && \
+    export WASM_BUILD_TOOLCHAIN=nightly-2021-02-20 && \
     BUILD_DUMMY_WASM_BINARY=1 cargo clippy --release --all -- -D warnings && \
     cargo test --release --all && \
     cargo build --release

runtime/src/proposals_configuration/mod.rs

@@ -4,10 +4,10 @@
 //! to be the integration tests.
 //!
 //! The whole parameter set is initialized only once by deserializing JSON from the environment variable
-//! "ALL_PROPOSALS_PARAMETERS_JSON". If it doesn't exists or contains invalid or empty JSON then
-//! the default parameters are returned. If some proposal section of the JSON file contains only
-//! partial object definition - default values are returned as missing fields.
-//!
+//! "ALL_PROPOSALS_PARAMETERS_JSON". If it doesn't exists the default parameters are returned.
+//! If some proposal section of the JSON file contains only
+//! partial object definition - default values are returned for missing fields.
+//! If passed JSON is invalid or expected numeric value is not a number it will panic!
 
 use crate::{Balance, BlockNumber, ProposalParameters};
 use frame_support::dispatch::Vec;
@@ -105,17 +105,20 @@ lazy_static! {
 #[allow(clippy::match_wild_err_arm)]
 // Composes AllProposalsParameters object from the JSON string.
 // It gets the JSON string from the environment variable and tries to parse it.
-// On error and any missing values it gets default values.
+// On error it will panic!
 fn get_all_proposals_parameters_objects() -> AllProposalsParameters {
     let json_str: Option<&'static str> = option_env!("ALL_PROPOSALS_PARAMETERS_JSON");
 
+    // Handle undefined variable (null) and variable set to empty string the same to work cross platform.
+    if json_str.map_or(true, str::is_empty) {
+        return default_parameters();
+    }
+
     json_str
         .map(lite_json::parse_json)
         .map(|res| match res {
             Ok(json) => Some(json),
-            Err(_) => {
-                panic!("Invalid JSON with proposals parameters provided.");
-            }
+            Err(_) => panic!("Invalid JSON with proposals parameters provided."),
         })
         .flatten()
         .map(convert_json_object_to_proposal_parameters)
@@ -278,7 +281,7 @@ fn extract_proposal_parameters(
     params
 }
 
-// Extracts a specific numeric parameter from the parsed JSON object.
+// Extracts a specific numeric parameter from the parsed JSON object. Will panic if expected value is not numeric.
 fn extract_numeric_parameter(
     json_object: &JsonValue,
     parameter_name: &'static str,

runtime/src/proposals_configuration/tests.rs

@@ -1,239 +1,5 @@
-use crate::{Balance, BlockNumber, ProposalParameters};
-
-fn default_proposal_parameters() -> ProposalParameters<BlockNumber, Balance> {
-    ProposalParameters {
-        voting_period: 1,
-        grace_period: 2,
-        approval_quorum_percentage: 3,
-        approval_threshold_percentage: 4,
-        slashing_quorum_percentage: 5,
-        slashing_threshold_percentage: 6,
-        required_stake: Some(7),
-        constitutionality: 8,
-    }
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_max_validators() {
-    let actual_params = super::SetMaxValidatorCountProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_runtime_upgrade() {
-    let actual_params = super::RuntimeUpgradeProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_signal() {
-    let actual_params = super::SignalProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_funding_request() {
-    let actual_params = super::FundingRequestProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_create_wg_lead_opening() {
-    let actual_params = super::CreateWorkingGroupLeadOpeningProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_wg_fill_lead_opening() {
-    let actual_params = super::FillWorkingGroupLeadOpeningProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_update_budget() {
-    let actual_params = super::UpdateWorkingGroupBudgetProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_decrease_wg_lead_stake() {
-    let actual_params = super::DecreaseWorkingGroupLeadStakeProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_slash_wg_lead() {
-    let actual_params = super::SlashWorkingGroupLeadProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_set_wg_lead_reward() {
-    let actual_params = super::SetWorkingGroupLeadRewardProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_terminate_wg_lead() {
-    let actual_params = super::TerminateWorkingGroupLeadProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_amend_constitution() {
-    let actual_params = super::AmendConstitutionProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_cancel_wg_lead_opening() {
-    let actual_params = super::CancelWorkingGroupLeadOpeningProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_set_membership_price() {
-    let actual_params = super::SetMembershipPriceProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_set_council_budget_increment() {
-    let actual_params = super::SetCouncilBudgetIncrementProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_set_councilor_reward() {
-    let actual_params = super::SetCouncilorRewardProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_set_initial_invitation_balance() {
-    let actual_params = super::SetInitialInvitationBalanceProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_set_membership_invitaiton_quota() {
-    let actual_params = super::SetMembershipLeadInvitationQuotaProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_set_referral_cut() {
-    let actual_params = super::SetReferralCutProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_set_invitation_count() {
-    let actual_params = super::SetInvitationCountProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_create_blog_post_proposal() {
-    let actual_params = super::CreateBlogPostProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_edit_blog_post_proposal() {
-    let actual_params = super::EditBlogPostProoposalParamters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_lock_blog_post_proposal() {
-    let actual_params = super::LockBlogPostProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
-#[test]
-#[ignore]
-fn proposal_parameters_are_initialized_unlock_blog_post_proposal() {
-    let actual_params = super::UnlockBlogPostProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
-}
-
-// Enable during the conditional compilation tests.
 #[test]
-#[ignore]
-fn proposal_parameters_are_initialized_veto_proposal_proposal() {
-    let actual_params = super::VetoProposalProposalParameters::get();
-
-    assert_eq!(default_proposal_parameters(), actual_params);
+fn proposal_parameters() {
+    // Reading all proposal parameters and look for invalid JSON
+    super::get_all_proposals_parameters_objects();
 }

runtime/src/tests/mod.rs

@@ -1,9 +1,11 @@
 //! The Joystream Substrate Node runtime integration tests.
-
 #![cfg(test)]
-#[macro_use]
-// Fix broken tests, partial fix done in - https://github.com/Joystream/joystream/pull/3252
+// TODO: Fix broken tests, partial fix done in - https://github.com/Joystream/joystream/pull/3252
+// Remove allow dead_code directive when re-enabling these tests
+#![allow(dead_code)]
+// #[macro_use]
 // mod proposals_integration;
+
 mod locks;
 
 // Temporary commented for Olympia: https://github.com/Joystream/joystream/issues/3237

scripts/cargo-build.sh

@@ -1,5 +1,7 @@
 #!/usr/bin/env bash
 
+# TODO: Check for valid JSON in ALL_PROPOSALS_PARAMETERS_JSON ?
+
 export WASM_BUILD_TOOLCHAIN=nightly-2021-02-20
 
 cargo +nightly-2021-02-20 build --release