Freelancer disputes (#296)

* allow initiator to raise dispute

* on_dispute_complete refactor, tests, test refactor

* fix

* fmt

* fix

pallets/disputes/src/lib.rs

@@ -62,7 +62,7 @@ pub mod pallet {
         /// The origin used to force cancel and pass disputes.
         type ForceOrigin: EnsureOrigin<Self::RuntimeOrigin>;
         /// External hooks to handle the completion of a dispute.
-        type DisputeHooks: DisputeHooks<Self::DisputeKey, Self::SpecificId>;
+        type DisputeHooks: DisputeHooks<Self::DisputeKey, Self::SpecificId, AccountIdOf<Self>>;
     }
 
     /// Used to store the disputes that is being raised, given the dispute key it returns the Dispute
@@ -145,8 +145,12 @@ pub mod pallet {
                     let hook_weight = <T::DisputeHooks as DisputeHooks<
                         T::DisputeKey,
                         T::SpecificId,
+                        T::AccountId,
                     >>::on_dispute_complete(
-                        *dispute_id, dispute.specifiers.into_inner(), result
+                        dispute.raised_by,
+                        *dispute_id,
+                        dispute.specifiers.into_inner(),
+                        result,
                     );
 
                     weight = weight.saturating_add(hook_weight);
@@ -216,6 +220,7 @@ pub mod pallet {
                 // Dont mind if this fails as the autofinalise will skip.
             });
             T::DisputeHooks::on_dispute_complete(
+                dispute.raised_by,
                 dispute_key,
                 dispute.specifiers.into_inner(),
                 DisputeResult::Failure,
@@ -248,6 +253,7 @@ pub mod pallet {
                 // Dont mind if this fails as the autofinalise will skip.
             });
             T::DisputeHooks::on_dispute_complete(
+                dispute.raised_by,
                 dispute_key,
                 dispute.specifiers.into_inner(),
                 DisputeResult::Success,
@@ -413,6 +419,7 @@ pub mod pallet {
 
             // Dont need to return the weight here.
             let _ = T::DisputeHooks::on_dispute_complete(
+                dispute.raised_by,
                 dispute_key,
                 dispute.specifiers.into_inner(),
                 result,

pallets/disputes/src/mock.rs

@@ -118,8 +118,9 @@ pub(crate) fn new_test_ext() -> sp_io::TestExternalities {
     ext
 }
 
-impl crate::traits::DisputeHooks<u32, u32> for Test {
+impl crate::traits::DisputeHooks<u32, u32, AccountId> for Test {
     fn on_dispute_complete(
+        _account_id: AccountId,
         _dispute_key: u32,
         _specifics: Vec<u32>,
         _dispute_result: crate::pallet::DisputeResult,

pallets/disputes/src/traits.rs

@@ -18,10 +18,11 @@ pub trait DisputeRaiser<AccountId> {
     ) -> Result<(), DispatchError>;
 }
 
-pub trait DisputeHooks<DisputeKey, SpecificId> {
+pub trait DisputeHooks<DisputeKey, SpecificId, AccountId> {
     /// On the completion of a dispute, this hooks is called.
     /// Returning only the key that has been handled and the result of the dispute.
     fn on_dispute_complete(
+        raised_by: AccountId,
         dispute_key: DisputeKey,
         specifics: Vec<SpecificId>,
         dispute_result: crate::pallet::DisputeResult,

pallets/grants/src/benchmarking.rs

@@ -5,7 +5,7 @@ use super::*;
 use crate::test_utils::gen_grant_id;
 use crate::Pallet as Grants;
 use crate::{BoundedApprovers, BoundedPMilestones, Config};
-use common_types::{CurrencyId, TreasuryOrigin, ForeignOwnedAccount};
+use common_types::{CurrencyId, ForeignOwnedAccount, TreasuryOrigin};
 use frame_benchmarking::v2::*;
 use frame_support::{assert_ok, traits::Get};
 use frame_system::pallet_prelude::BlockNumberFor;

pallets/proposals/src/benchmarking.rs

@@ -220,7 +220,8 @@ mod benchmarks {
             project_key,
             milestone_keys.clone()
         ));
-        let _ = <crate::Pallet<T> as DisputeHooks<ProjectKey, MilestoneKey>>::on_dispute_complete(
+        let _ = <crate::Pallet<T> as DisputeHooks<ProjectKey, MilestoneKey, AccountIdOf<T>>>::on_dispute_complete(
+            bob.clone(),
             project_key,
             milestone_keys.into_inner(),
             DisputeResult::Success,

pallets/proposals/src/impls/pallet_impls.rs

@@ -337,8 +337,9 @@ impl<T: Config> Pallet<T> {
     }
 }
 
-impl<T: Config> DisputeHooks<ProjectKey, MilestoneKey> for Pallet<T> {
+impl<T: Config> DisputeHooks<ProjectKey, MilestoneKey, AccountIdOf<T>> for Pallet<T> {
     fn on_dispute_complete(
+        raised_by: AccountIdOf<T>,
         project_key: ProjectKey,
         specifics: Vec<MilestoneKey>,
         dispute_result: pallet_disputes::pallet::DisputeResult,
@@ -354,7 +355,11 @@ impl<T: Config> DisputeHooks<ProjectKey, MilestoneKey> for Pallet<T> {
                                     // Shouldnt be needed but nice to have this check.
                                     // Will prevent someone calling both refund and withdraw on the same milestone.
                                     if milestone.transfer_status.is_none() {
-                                        milestone.can_refund = true;
+                                        if project.initiator == raised_by {
+                                            milestone.is_approved = true;
+                                        } else {
+                                            milestone.can_refund = true;
+                                        }
                                     }
                                 }
                             }

pallets/proposals/src/lib.rs

@@ -344,14 +344,14 @@ pub mod pallet {
         TooManyMilestoneVotes,
         /// An internal error, a collection of votes for a milestone has been lost.s
         IndividualVoteNotFound,
-        /// Only a contributor can raise a dispute.
-        OnlyContributorsCanRaiseDispute,
+        /// Only a contributor can initiate a refund.
+        OnlyContributorsCanInitiateRefund,
         /// One of these milestones is already in a dispute.
         MilestonesAlreadyInDispute,
         /// You cannot raise a dispute on an approved milestone.
         CannotRaiseDisputeOnApprovedMilestone,
-        /// Only a contributor can initiate a refund.
-        OnlyContributorsCanInitiateRefund,
+        /// Only a contributor or initiator can initiate a refund.
+        NotPermittedToRaiseDispute,
         /// Only the ForeignAssetSigner can mint tokens
         RequireForeignAssetSigner,
         /// A Jury is required to create a project.
@@ -456,9 +456,10 @@ pub mod pallet {
                 Error::<T>::MilestoneDoesNotExist
             );
             ensure!(
-                project.contributions.contains_key(&who),
-                Error::<T>::OnlyContributorsCanRaiseDispute
+                project.contributions.contains_key(&who) || &who == &project.initiator,
+                Error::<T>::NotPermittedToRaiseDispute
             );
+
             ensure!(
                 !ProjectsInDispute::<T>::contains_key(project_key),
                 Error::<T>::MilestonesAlreadyInDispute
@@ -472,7 +473,16 @@ pub mod pallet {
 
             if project.jury.len() == 1 {
                 // https://github.com/ImbueNetwork/imbue/issues/270
-                let _ = <Self as pallet_disputes::traits::DisputeHooks<ProjectKey, MilestoneKey>>::on_dispute_complete(project_key, milestone_keys.to_vec(), pallet_disputes::DisputeResult::Success);
+                let _ = <Self as pallet_disputes::traits::DisputeHooks<
+                    ProjectKey,
+                    MilestoneKey,
+                    AccountIdOf<T>,
+                >>::on_dispute_complete(
+                    who,
+                    project_key,
+                    milestone_keys.to_vec(),
+                    pallet_disputes::DisputeResult::Success,
+                );
             } else {
                 <T as Config>::DisputeRaiser::raise_dispute(
                     project_key,

pallets/proposals/src/test_utils.rs

@@ -140,11 +140,12 @@ pub fn create_funded_user<T: Config>(
 
 /// Manually call the hook OnDisputeCompleteWith a predefined result for testing>
 pub fn complete_dispute<T: Config>(
+    dispute_raised_by: T::AccountId,
     project_key: ProjectKey,
     milestone_keys: Vec<MilestoneKey>,
     result: pallet_disputes::DisputeResult,
 ) -> crate::Weight {
-    <crate::Pallet<T>>::on_dispute_complete(project_key, milestone_keys, result)
+    <crate::Pallet<T>>::on_dispute_complete(dispute_raised_by, project_key, milestone_keys, result)
 }
 
 pub fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {

pallets/proposals/src/tests/dispute_approvals.rs

@@ -0,0 +1,54 @@
+use crate::{mock::*, *};
+use frame_support::{assert_noop, assert_ok};
+use pallet_disputes::DisputeResult;
+use test_utils::*;
+
+#[test]
+fn initiator_dispute_complete_sets_milestones_to_approved() {
+    build_test_externality().execute_with(|| {
+        let per_contribution = 100000u128;
+        let contributions = get_contributions::<Test>(vec![BOB, CHARLIE], per_contribution);
+        let milestones = get_milestones(10);
+        let jury = vec![JURY_1, JURY_2];
+        let initiator = ALICE;
+
+        let project_key = create_and_fund_project::<Test>(
+            ALICE,
+            contributions,
+            milestones.clone(),
+            CurrencyId::Native,
+            jury,
+        )
+        .unwrap();
+        let milestone_keys: BoundedVec<u32, <Test as Config>::MaxMilestonesPerProject> = (1u32
+            ..milestones.len() as u32)
+            .collect::<Vec<u32>>()
+            .try_into()
+            .unwrap();
+
+        assert_ok!(Proposals::raise_dispute(
+            RuntimeOrigin::signed(initiator),
+            project_key,
+            milestone_keys.clone()
+        ));
+
+        let _ = complete_dispute::<Test>(
+            initiator,
+            project_key,
+            milestone_keys.into_inner(),
+            DisputeResult::Success,
+        );
+
+        let project = Projects::<Test>::get(project_key).unwrap();
+
+        for milestone in project.milestones.iter().for_each(|(key, milestone)|{
+            if milestone_keys.contains(&key) {
+                assert!(milestone.is_approved, "dispute success for initiator should approve milestones.")
+            } else {
+                assert!(!milestone.is_approved, "other milestones should be left unapproved.")
+            }
+        })
+    })
+}
+
+

pallets/proposals/src/tests/disputes.rs

@@ -5,7 +5,7 @@ use pallet_disputes::DisputeResult;
 use test_utils::*;
 
 #[test]
-fn raise_dispute_not_contributor() {
+fn raise_dispute_not_contributor_or_initiator() {
     build_test_externality().execute_with(|| {
         let contributions = get_contributions::<Test>(vec![BOB, CHARLIE], 1_000_000u128);
         let milestones = get_milestones(10);
@@ -27,7 +27,7 @@ fn raise_dispute_not_contributor() {
 
         assert_noop!(
             Proposals::raise_dispute(RuntimeOrigin::signed(JOHN), project_key, milestone_keys),
-            Error::<Test>::OnlyContributorsCanRaiseDispute
+            Error::<Test>::NotPermittedToRaiseDispute
         );
     })
 }
@@ -206,6 +206,7 @@ fn on_dispute_complete_success_removes_dispute_status() {
             milestone_keys.clone()
         ));
         let _ = complete_dispute::<Test>(
+            BOB,
             project_key,
             milestone_keys.into_inner(),
             DisputeResult::Success,
@@ -240,6 +241,7 @@ fn on_dispute_complete_failure_removes_dispute_status() {
             milestone_keys.clone()
         ));
         let _ = complete_dispute::<Test>(
+            BOB,
             project_key,
             milestone_keys.into_inner(),
             DisputeResult::Failure,
@@ -275,6 +277,7 @@ fn dispute_success_does_not_cancel_project() {
             milestone_keys.clone()
         ));
         let _ = complete_dispute::<Test>(
+            BOB,
             project_key,
             milestone_keys.into_inner(),
             DisputeResult::Success,
@@ -312,6 +315,7 @@ fn dispute_success_approves_milestone_for_refund_but_only_ones_specified() {
             milestone_keys.clone()
         ));
         let _ = complete_dispute::<Test>(
+            BOB,
             project_key,
             milestone_keys.into_inner(),
             DisputeResult::Success,
@@ -485,6 +489,7 @@ fn failed_dispute_tests() {
             dispute_milestone_keys.clone()
         ));
         let _ = complete_dispute::<Test>(
+            BOB,
             project_key,
             dispute_milestone_keys.into_inner(),
             DisputeResult::Failure,
@@ -525,6 +530,7 @@ fn assert_can_recall_dispute_after_success() {
             milestone_keys.clone()
         ));
         let _ = complete_dispute::<Test>(
+            BOB,
             project_key,
             milestone_keys.into_inner(),
             DisputeResult::Success,
@@ -564,6 +570,7 @@ fn assert_can_recall_dispute_after_failure() {
             milestone_keys.clone()
         ));
         let _ = complete_dispute::<Test>(
+            BOB,
             project_key,
             milestone_keys.into_inner(),
             DisputeResult::Failure,
@@ -627,3 +634,61 @@ fn raise_dispute_with_single_jury_auto_completes() {
         });
     })
 }
+
+#[test]
+fn raise_dispute_as_initiator_success() {
+    build_test_externality().execute_with(|| {
+        let contributions = get_contributions::<Test>(vec![BOB, CHARLIE], 1_000_000u128);
+        let milestones = get_milestones(10);
+        let jury = vec![JURY_1, JURY_2];
+
+        let project_key = create_and_fund_project::<Test>(
+            ALICE,
+            contributions,
+            milestones.clone(),
+            CurrencyId::Native,
+            jury,
+        )
+        .unwrap();
+
+        let dispute_milestone_keys: BoundedVec<u32, <Test as Config>::MaxMilestonesPerProject> =
+            (0u32..milestones.len() as u32)
+                .collect::<Vec<u32>>()
+                .try_into()
+                .unwrap();
+        assert_ok!(Proposals::raise_dispute(
+            RuntimeOrigin::signed(ALICE),
+            project_key,
+            dispute_milestone_keys
+        ));
+    })
+}
+
+#[test]
+fn raise_dispute_as_contributor_success() {
+    build_test_externality().execute_with(|| {
+        let contributions = get_contributions::<Test>(vec![BOB, CHARLIE], 1_000_000u128);
+        let milestones = get_milestones(10);
+        let jury = vec![JURY_1, JURY_2];
+
+        let project_key = create_and_fund_project::<Test>(
+            ALICE,
+            contributions,
+            milestones.clone(),
+            CurrencyId::Native,
+            jury,
+        )
+        .unwrap();
+
+        let dispute_milestone_keys: BoundedVec<u32, <Test as Config>::MaxMilestonesPerProject> =
+            (0u32..milestones.len() as u32)
+                .collect::<Vec<u32>>()
+                .try_into()
+                .unwrap();
+        assert_ok!(Proposals::raise_dispute(
+            RuntimeOrigin::signed(BOB),
+            project_key,
+            dispute_milestone_keys
+        ));
+    })
+}