Merge pull request #4644 from Lezek123/tweak-remarks

Tweak remarks

bin/node/Cargo.toml

@@ -3,7 +3,7 @@ authors = ['Joystream contributors']
 build = 'build.rs'
 edition = '2018'
 name = 'joystream-node'
-version = '8.2.0'
+version = '8.3.0'
 default-run = "joystream-node"
 
 [[bin]]

bin/utils/chain-spec-builder/Cargo.toml

@@ -3,7 +3,7 @@ authors = ['Joystream contributors']
 build = 'build.rs'
 edition = '2018'
 name = 'chain-spec-builder'
-version = '8.2.0'
+version = '8.3.0'
 
 [dependencies]
 enum-utils = "0.1.2"

runtime-modules/working-group/src/lib.rs

@@ -1215,7 +1215,7 @@ decl_module! {
         /// # </weight>
         #[weight = WeightInfoWorkingGroup::<T,I>::lead_remark(to_kb(msg.len() as u32))]
         pub fn lead_remark(origin, msg: Vec<u8>) {
-            let _ = checks::ensure_origin_is_active_leader::<T, I>(origin);
+            checks::ensure_origin_is_active_leader::<T, I>(origin)?;
 
             //
             // == MUTATION SAFE ==
@@ -1236,7 +1236,7 @@ decl_module! {
         /// # </weight>
         #[weight = WeightInfoWorkingGroup::<T,I>::worker_remark(to_kb(msg.len() as u32))]
         pub fn worker_remark(origin, worker_id: WorkerId<T>,msg: Vec<u8>) {
-            let _ = checks::ensure_worker_signed::<T, I>(origin, &worker_id).map(|_| ());
+            checks::ensure_worker_signed::<T, I>(origin, &worker_id)?;
 
             //
             // == MUTATION SAFE ==

runtime-modules/working-group/src/tests/fixtures.rs

@@ -760,7 +760,7 @@ pub(crate) fn get_stake_balance(account_id: &u64) -> u64 {
     existing_lock.map_or(0, |lock| lock.amount)
 }
 
-fn get_current_lead_account_id() -> u64 {
+pub(crate) fn get_current_lead_account_id() -> u64 {
     let leader_worker_id = TestWorkingGroup::current_lead();
 
     if let Some(leader_worker_id) = leader_worker_id {

runtime-modules/working-group/src/tests/mod.rs

@@ -7,9 +7,9 @@ pub use mock::{build_test_externalities, Test, DEFAULT_WORKER_ACCOUNT_ID};
 use frame_system::RawOrigin;
 
 use crate::tests::fixtures::{
-    set_invitation_lock, CancelOpeningFixture, DecreaseWorkerStakeFixture,
-    FundWorkingGroupBudgetFixture, IncreaseWorkerStakeFixture, SetBudgetFixture,
-    SetStatusTextFixture, SlashWorkerStakeFixture, SpendFromBudgetFixture,
+    get_current_lead_account_id, set_invitation_lock, CancelOpeningFixture,
+    DecreaseWorkerStakeFixture, FundWorkingGroupBudgetFixture, IncreaseWorkerStakeFixture,
+    SetBudgetFixture, SetStatusTextFixture, SlashWorkerStakeFixture, SpendFromBudgetFixture,
     UpdateRewardAccountFixture, UpdateRewardAmountFixture, WithdrawApplicationFixture,
 };
 use crate::tests::hiring_workflow::HiringWorkflow;
@@ -29,6 +29,7 @@ use fixtures::{
 use frame_support::dispatch::DispatchError;
 use frame_support::traits::Currency;
 use frame_support::StorageMap;
+use frame_support::{assert_noop, assert_ok};
 use mock::{run_to_block, Balances, RewardPeriod, TestWorkingGroup, ACTOR_ORIGIN_ERROR};
 use sp_runtime::traits::Hash;
 use sp_std::collections::btree_map::BTreeMap;
@@ -2642,3 +2643,121 @@ fn fund_wg_budget_fails_with_zero_amount() {
             .call_and_assert(Err(Error::<Test, DefaultInstance>::ZeroTokensFunding.into()));
     });
 }
+
+#[test]
+fn lead_remark_fails_with_invalid_origin() {
+    build_test_externalities().execute_with(|| {
+        run_to_block(1);
+        HireLeadFixture::default().hire_lead();
+        assert_noop!(
+            TestWorkingGroup::lead_remark(RawOrigin::None.into(), Vec::new()),
+            DispatchError::BadOrigin
+        );
+    });
+}
+
+#[test]
+fn lead_remark_fails_when_lead_not_hired() {
+    build_test_externalities().execute_with(|| {
+        run_to_block(1);
+        let account_id = 0u64;
+        assert_noop!(
+            TestWorkingGroup::lead_remark(RawOrigin::Signed(account_id).into(), Vec::new()),
+            Error::<Test, DefaultInstance>::CurrentLeadNotSet
+        );
+    });
+}
+
+#[test]
+fn lead_remark_fails_with_invalid_signer() {
+    build_test_externalities().execute_with(|| {
+        run_to_block(1);
+        let worker_id = HireRegularWorkerFixture::default().hire();
+        let worker_acc = TestWorkingGroup::worker_by_id(worker_id)
+            .unwrap()
+            .role_account_id;
+        assert_noop!(
+            TestWorkingGroup::lead_remark(RawOrigin::Signed(worker_acc).into(), Vec::new()),
+            Error::<Test, DefaultInstance>::IsNotLeadAccount
+        );
+    });
+}
+
+#[test]
+fn lead_remark_ok() {
+    build_test_externalities().execute_with(|| {
+        run_to_block(1);
+        HireLeadFixture::default().hire_lead();
+        let lead_acc = get_current_lead_account_id();
+        assert_ok!(TestWorkingGroup::lead_remark(
+            RawOrigin::Signed(lead_acc).into(),
+            Vec::new()
+        ));
+        EventFixture::assert_last_crate_event(RawEvent::LeadRemarked(Vec::new()));
+    });
+}
+
+#[test]
+fn worker_remark_fails_with_invalid_origin() {
+    build_test_externalities().execute_with(|| {
+        run_to_block(1);
+        let worker_id = HireRegularWorkerFixture::default().hire();
+        assert_noop!(
+            TestWorkingGroup::worker_remark(RawOrigin::None.into(), worker_id, Vec::new()),
+            DispatchError::BadOrigin
+        );
+    });
+}
+
+#[test]
+fn worker_remark_fails_with_invalid_worker_id() {
+    build_test_externalities().execute_with(|| {
+        run_to_block(1);
+        let worker_id = HireRegularWorkerFixture::default().hire();
+        let worker_acc = TestWorkingGroup::worker_by_id(worker_id)
+            .unwrap()
+            .role_account_id;
+        assert_noop!(
+            TestWorkingGroup::worker_remark(
+                RawOrigin::Signed(worker_acc).into(),
+                worker_id + 1,
+                Vec::new()
+            ),
+            Error::<Test, DefaultInstance>::WorkerDoesNotExist
+        );
+    });
+}
+
+#[test]
+fn worker_remark_fails_with_invalid_signer() {
+    build_test_externalities().execute_with(|| {
+        run_to_block(1);
+        let worker_id = HireRegularWorkerFixture::default().hire();
+        let lead_acc = get_current_lead_account_id();
+        assert_noop!(
+            TestWorkingGroup::worker_remark(
+                RawOrigin::Signed(lead_acc).into(),
+                worker_id,
+                Vec::new()
+            ),
+            Error::<Test, DefaultInstance>::SignerIsNotWorkerRoleAccount
+        );
+    });
+}
+
+#[test]
+fn worker_remark_ok() {
+    build_test_externalities().execute_with(|| {
+        run_to_block(1);
+        let worker_id = HireRegularWorkerFixture::default().hire();
+        let worker_acc = TestWorkingGroup::worker_by_id(worker_id)
+            .unwrap()
+            .role_account_id;
+        assert_ok!(TestWorkingGroup::worker_remark(
+            RawOrigin::Signed(worker_acc).into(),
+            worker_id,
+            Vec::new()
+        ));
+        EventFixture::assert_last_crate_event(RawEvent::WorkerRemarked(worker_id, Vec::new()));
+    });
+}

runtime/Cargo.toml

@@ -4,7 +4,7 @@ edition = '2018'
 name = 'joystream-node-runtime'
 # Follow convention: https://github.com/Joystream/substrate-runtime-joystream/issues/1
 # {Authoring}.{Spec}.{Impl} of the RuntimeVersion
-version = '12.2000.0'
+version = '12.2001.0'
 
 [dependencies]
 # Third-party dependencies

runtime/src/lib.rs

@@ -145,7 +145,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
     spec_name: create_runtime_str!("joystream-node"),
     impl_name: create_runtime_str!("joystream-node"),
     authoring_version: 12,
-    spec_version: 2000,
+    spec_version: 2001,
     impl_version: 0,
     apis: crate::runtime_api::EXPORTED_RUNTIME_API_VERSIONS,
     transaction_version: 1,