Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Freelancer disputes #296

Merged
merged 5 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions pallets/disputes/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion pallets/disputes/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion pallets/disputes/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion pallets/grants/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion pallets/proposals/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 7 additions & 2 deletions pallets/proposals/src/impls/pallet_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
}
}
}
}
Expand Down
24 changes: 17 additions & 7 deletions pallets/proposals/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion pallets/proposals/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
54 changes: 54 additions & 0 deletions pallets/proposals/src/tests/dispute_approvals.rs
Original file line number Diff line number Diff line change
@@ -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.")
}
})
})
}


69 changes: 67 additions & 2 deletions pallets/proposals/src/tests/disputes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
);
})
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
));
})
}
Loading
Loading