diff --git a/pallets/disputes/src/lib.rs b/pallets/disputes/src/lib.rs index 31e250ea..98e266d0 100644 --- a/pallets/disputes/src/lib.rs +++ b/pallets/disputes/src/lib.rs @@ -62,7 +62,7 @@ pub mod pallet { /// The origin used to force cancel and pass disputes. type ForceOrigin: EnsureOrigin; /// External hooks to handle the completion of a dispute. - type DisputeHooks: DisputeHooks; + type DisputeHooks: DisputeHooks>; } /// 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 = >::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, diff --git a/pallets/disputes/src/mock.rs b/pallets/disputes/src/mock.rs index 2651566a..af4ad327 100644 --- a/pallets/disputes/src/mock.rs +++ b/pallets/disputes/src/mock.rs @@ -118,8 +118,9 @@ pub(crate) fn new_test_ext() -> sp_io::TestExternalities { ext } -impl crate::traits::DisputeHooks for Test { +impl crate::traits::DisputeHooks for Test { fn on_dispute_complete( + _account_id: AccountId, _dispute_key: u32, _specifics: Vec, _dispute_result: crate::pallet::DisputeResult, diff --git a/pallets/disputes/src/traits.rs b/pallets/disputes/src/traits.rs index 1f033cf5..387e697b 100644 --- a/pallets/disputes/src/traits.rs +++ b/pallets/disputes/src/traits.rs @@ -18,10 +18,11 @@ pub trait DisputeRaiser { ) -> Result<(), DispatchError>; } -pub trait DisputeHooks { +pub trait DisputeHooks { /// 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, dispute_result: crate::pallet::DisputeResult, diff --git a/pallets/grants/src/benchmarking.rs b/pallets/grants/src/benchmarking.rs index eafa9410..ce780d5e 100644 --- a/pallets/grants/src/benchmarking.rs +++ b/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; diff --git a/pallets/proposals/src/benchmarking.rs b/pallets/proposals/src/benchmarking.rs index be1f3a07..8005f347 100644 --- a/pallets/proposals/src/benchmarking.rs +++ b/pallets/proposals/src/benchmarking.rs @@ -220,7 +220,8 @@ mod benchmarks { project_key, milestone_keys.clone() )); - let _ = as DisputeHooks>::on_dispute_complete( + let _ = as DisputeHooks>>::on_dispute_complete( + bob.clone(), project_key, milestone_keys.into_inner(), DisputeResult::Success, diff --git a/pallets/proposals/src/impls/pallet_impls.rs b/pallets/proposals/src/impls/pallet_impls.rs index 6c126738..636212ce 100644 --- a/pallets/proposals/src/impls/pallet_impls.rs +++ b/pallets/proposals/src/impls/pallet_impls.rs @@ -337,8 +337,9 @@ impl Pallet { } } -impl DisputeHooks for Pallet { +impl DisputeHooks> for Pallet { fn on_dispute_complete( + raised_by: AccountIdOf, project_key: ProjectKey, specifics: Vec, dispute_result: pallet_disputes::pallet::DisputeResult, @@ -354,7 +355,11 @@ impl DisputeHooks for Pallet { // 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; + } } } } diff --git a/pallets/proposals/src/lib.rs b/pallets/proposals/src/lib.rs index ac350a70..31f5c666 100644 --- a/pallets/proposals/src/lib.rs +++ b/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::::MilestoneDoesNotExist ); ensure!( - project.contributions.contains_key(&who), - Error::::OnlyContributorsCanRaiseDispute + project.contributions.contains_key(&who) || &who == &project.initiator, + Error::::NotPermittedToRaiseDispute ); + ensure!( !ProjectsInDispute::::contains_key(project_key), Error::::MilestonesAlreadyInDispute @@ -472,7 +473,16 @@ pub mod pallet { if project.jury.len() == 1 { // https://github.com/ImbueNetwork/imbue/issues/270 - let _ = >::on_dispute_complete(project_key, milestone_keys.to_vec(), pallet_disputes::DisputeResult::Success); + let _ = , + >>::on_dispute_complete( + who, + project_key, + milestone_keys.to_vec(), + pallet_disputes::DisputeResult::Success, + ); } else { ::DisputeRaiser::raise_dispute( project_key, diff --git a/pallets/proposals/src/test_utils.rs b/pallets/proposals/src/test_utils.rs index 6803a5d9..f9995202 100644 --- a/pallets/proposals/src/test_utils.rs +++ b/pallets/proposals/src/test_utils.rs @@ -140,11 +140,12 @@ pub fn create_funded_user( /// Manually call the hook OnDisputeCompleteWith a predefined result for testing> pub fn complete_dispute( + dispute_raised_by: T::AccountId, project_key: ProjectKey, milestone_keys: Vec, result: pallet_disputes::DisputeResult, ) -> crate::Weight { - >::on_dispute_complete(project_key, milestone_keys, result) + >::on_dispute_complete(dispute_raised_by, project_key, milestone_keys, result) } pub fn assert_last_event(generic_event: ::RuntimeEvent) { diff --git a/pallets/proposals/src/tests/dispute_approvals.rs b/pallets/proposals/src/tests/dispute_approvals.rs new file mode 100644 index 00000000..87ef9c85 --- /dev/null +++ b/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::(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::( + ALICE, + contributions, + milestones.clone(), + CurrencyId::Native, + jury, + ) + .unwrap(); + let milestone_keys: BoundedVec::MaxMilestonesPerProject> = (1u32 + ..milestones.len() as u32) + .collect::>() + .try_into() + .unwrap(); + + assert_ok!(Proposals::raise_dispute( + RuntimeOrigin::signed(initiator), + project_key, + milestone_keys.clone() + )); + + let _ = complete_dispute::( + initiator, + project_key, + milestone_keys.into_inner(), + DisputeResult::Success, + ); + + let project = Projects::::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.") + } + }) + }) +} + + diff --git a/pallets/proposals/src/tests/disputes.rs b/pallets/proposals/src/tests/disputes.rs index 87a76afe..1c34d360 100644 --- a/pallets/proposals/src/tests/disputes.rs +++ b/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::(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::::OnlyContributorsCanRaiseDispute + Error::::NotPermittedToRaiseDispute ); }) } @@ -206,6 +206,7 @@ fn on_dispute_complete_success_removes_dispute_status() { milestone_keys.clone() )); let _ = complete_dispute::( + 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::( + 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::( + 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::( + BOB, project_key, milestone_keys.into_inner(), DisputeResult::Success, @@ -485,6 +489,7 @@ fn failed_dispute_tests() { dispute_milestone_keys.clone() )); let _ = complete_dispute::( + 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::( + 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::( + 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::(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::( + ALICE, + contributions, + milestones.clone(), + CurrencyId::Native, + jury, + ) + .unwrap(); + + let dispute_milestone_keys: BoundedVec::MaxMilestonesPerProject> = + (0u32..milestones.len() as u32) + .collect::>() + .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::(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::( + ALICE, + contributions, + milestones.clone(), + CurrencyId::Native, + jury, + ) + .unwrap(); + + let dispute_milestone_keys: BoundedVec::MaxMilestonesPerProject> = + (0u32..milestones.len() as u32) + .collect::>() + .try_into() + .unwrap(); + assert_ok!(Proposals::raise_dispute( + RuntimeOrigin::signed(BOB), + project_key, + dispute_milestone_keys + )); + }) +} diff --git a/pallets/proposals/src/tests/refunds.rs b/pallets/proposals/src/tests/refunds.rs index bca95eec..8633aa7f 100644 --- a/pallets/proposals/src/tests/refunds.rs +++ b/pallets/proposals/src/tests/refunds.rs @@ -29,6 +29,7 @@ fn you_can_actually_refund_after_dispute_success() { milestone_keys.clone() )); let _ = complete_dispute::( + BOB, project_key, milestone_keys.into_inner(), DisputeResult::Success, @@ -63,6 +64,7 @@ fn refund_assert_milestone_state_change() { milestone_keys.clone() )); let _ = complete_dispute::( + BOB, project_key, milestone_keys.into_inner(), DisputeResult::Success, @@ -116,6 +118,7 @@ fn refund_not_contributor() { milestone_keys.clone() )); let _ = complete_dispute::( + BOB, project_key, milestone_keys.into_inner(), DisputeResult::Success, @@ -153,6 +156,7 @@ fn refund_deletes_project_when_all_funds_are_refunded() { milestone_keys.clone() )); let _ = complete_dispute::( + BOB, project_key, milestone_keys.into_inner(), DisputeResult::Success, @@ -198,6 +202,7 @@ fn withdraw_then_refund_no_double_spend() { milestone_keys.clone(), ); let _ = complete_dispute::( + BOB, project_key, milestone_keys.into_inner(), DisputeResult::Success, @@ -268,6 +273,7 @@ fn refund_then_withdraw_no_double_spend() { milestone_keys.clone(), ); let _ = complete_dispute::( + BOB, project_key, milestone_keys.into_inner(), DisputeResult::Success, @@ -322,6 +328,7 @@ fn refund_check_refund_amount() { milestone_keys.clone() )); let _ = complete_dispute::( + BOB, project_key, milestone_keys.into_inner(), DisputeResult::Success, @@ -383,6 +390,7 @@ fn refund_takes_imbue_fee() { milestone_keys.clone() )); let _ = complete_dispute::( + BOB, project_key, milestone_keys.into_inner(), DisputeResult::Success,