diff --git a/pallets/disputes/src/lib.rs b/pallets/disputes/src/lib.rs index 31e250ea..220ca7dd 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,9 @@ 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 +217,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 +250,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 +416,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/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/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 fc410063..7d40588c 100644 --- a/pallets/proposals/src/lib.rs +++ b/pallets/proposals/src/lib.rs @@ -344,8 +344,8 @@ 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. @@ -456,10 +456,10 @@ pub mod pallet { Error::::MilestoneDoesNotExist ); ensure!( - project.contributions.contains_key(&who) || &who == project.initiator, + project.contributions.contains_key(&who) || &who == &project.initiator, Error::::NotPermittedToRaiseDispute ); - + ensure!( !ProjectsInDispute::::contains_key(project_key), Error::::MilestonesAlreadyInDispute @@ -473,7 +473,7 @@ 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 9ee55e37..af723b0c 100644 --- a/pallets/proposals/src/tests/disputes.rs +++ b/pallets/proposals/src/tests/disputes.rs @@ -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, @@ -633,7 +640,6 @@ 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 milestone_key = 0; let jury = vec![JURY_1, JURY_2]; let project_key = create_and_fund_project::( @@ -663,7 +669,6 @@ 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 milestone_key = 0; let jury = vec![JURY_1, JURY_2]; let project_key = create_and_fund_project::( diff --git a/pallets/proposals/src/tests/refunds.rs b/pallets/proposals/src/tests/refunds.rs index bca95eec..b4834bf2 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, @@ -419,3 +427,4 @@ fn refund_takes_imbue_fee() { ); }) } +