Skip to content

Commit

Permalink
MESH-2145/allow-the-owner-to-reject-proposal (#1632)
Browse files Browse the repository at this point in the history 
* Allows the owner of the proposal to change their vote if no one else has voted

* Add ProposalFailedToExecute event

* Emit old event

---------

Co-authored-by: Robert Gabriel Jakabosky <rjakabosky+neopallium@neoawareness.com>
Co-authored-by: Adam Dossa <adam.dossa@gmail.com>
  • Loading branch information
@HenriqueNogara @Neopallium @adamdossa
3 people committed Apr 11, 2024
1 parent 2a26588 commit e3f6819
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 7 deletions.
3 changes: 3 additions & 0 deletions pallets/common/src/traits/multisig.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ decl_event!(
ProposalExecutionFailed(DispatchError),
/// Scheduling of proposal fails.
SchedulingFailed(DispatchError),
/// Event emitted when a proposal failed to execute.
/// Arguments: caller DID, multisig, proposal ID, error.
ProposalFailedToExecute(IdentityId, AccountId, u64, DispatchError),
}
);

Expand Down
28 changes: 23 additions & 5 deletions pallets/multisig/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,12 @@ impl<T: Config> Module<T> {
Err(e) => {
update_proposal_status(ProposalStatus::ExecutionFailed);
Self::deposit_event(RawEvent::ProposalExecutionFailed(e.error));
Self::deposit_event(RawEvent::ProposalFailedToExecute(
multisig_did,
multisig.clone(),
proposal_id,
e.error,
));
false
}
};
Expand All @@ -905,11 +911,18 @@ impl<T: Config> Module<T> {
proposal_id: u64,
) -> DispatchResult {
Self::ensure_ms_signer(&multisig, &signer)?;
ensure!(
!Self::votes((&multisig, proposal_id), &signer),
Error::<T>::AlreadyVoted
);

let mut proposal_details = Self::proposal_detail(&multisig, proposal_id);

// Only allow the original proposer to change their vote if no one else has voted
let mut proposal_owner = false;
if Votes::<T>::get((&multisig, proposal_id), &signer) {
if proposal_details.rejections != 0 || proposal_details.approvals != 1 {
return Err(Error::<T>::AlreadyVoted.into());
}
proposal_owner = true;
}

proposal_details.rejections += 1u64;
let current_did = Context::current_identity::<Identity<T>>().unwrap_or_default();
match proposal_details.status {
Expand All @@ -929,7 +942,12 @@ impl<T: Config> Module<T> {
if proposal_details.auto_close {
let approvals_needed = Self::ms_signs_required(multisig.clone());
let ms_signers = Self::number_of_signers(multisig.clone());
if proposal_details.rejections > ms_signers.saturating_sub(approvals_needed) {
if proposal_details.rejections > ms_signers.saturating_sub(approvals_needed)
|| proposal_owner
{
if proposal_owner {
proposal_details.approvals = 0;
}
proposal_details.status = ProposalStatus::Rejected;
Self::deposit_event(RawEvent::ProposalRejected(
current_did,
Expand Down
140 changes: 138 additions & 2 deletions pallets/runtime/tests/src/multisig.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use frame_support::{assert_noop, assert_ok, StorageMap};
use frame_support::{assert_noop, assert_ok, StorageDoubleMap, StorageMap};

use pallet_multisig::{self as multisig, LostCreatorPrivileges};
use pallet_multisig::{self as multisig, LostCreatorPrivileges, ProposalDetail, Votes};
use polymesh_common_utilities::constants::currency::POLY;
use polymesh_primitives::multisig::ProposalStatus;
use polymesh_primitives::{AccountId, AuthorizationData, Permissions, SecondaryKey, Signatory};
Expand Down Expand Up @@ -1240,6 +1240,142 @@ fn remove_creator_controls_successfully() {
});
}

#[test]
fn proposal_owner_rejection() {
ExtBuilder::default().build().execute_with(|| {
let eve: User = User::new(AccountKeyring::Eve);
let bob: User = User::new(AccountKeyring::Bob);
let dave: User = User::new(AccountKeyring::Dave);
let alice: User = User::new(AccountKeyring::Alice);

// Creates a multi-signature
let ms_address =
MultiSig::get_next_multisig_address(AccountKeyring::Alice.to_account_id()).unwrap();
setup_multisig(
alice.origin(),
3,
vec![
Signatory::from(alice.did),
Signatory::from(bob.did),
Signatory::from(dave.did),
Signatory::from(eve.did),
],
);

// Creates a proposal
let call1 = Box::new(RuntimeCall::MultiSig(
multisig::Call::change_sigs_required { sigs_required: 4 },
));
assert_ok!(MultiSig::create_proposal_as_identity(
alice.origin(),
ms_address.clone(),
call1,
None,
true
));
let proposal_id = MultiSig::ms_tx_done(ms_address.clone()) - 1;

// The owner of the proposal should be able to reject it if no one else has voted
assert_ok!(MultiSig::reject_as_identity(
alice.origin(),
ms_address.clone(),
proposal_id
));

// The proposal status must be set to rejected
let proposal_details = ProposalDetail::<TestStorage>::get(&ms_address, proposal_id);
assert_eq!(proposal_details.status, ProposalStatus::Rejected);
assert_eq!(proposal_details.approvals, 0);
assert_eq!(proposal_details.rejections, 1);
assert_eq!(proposal_details.auto_close, true);
assert_eq!(
Votes::<TestStorage>::get((&ms_address, proposal_id), Signatory::from(alice.did)),
true
);

// The owner shouldn't be able to change their vote again
assert_noop!(
MultiSig::reject_as_identity(alice.origin(), ms_address.clone(), proposal_id),
Error::AlreadyVoted
);

// No other votes are allowed, since the proposal has been rejected
assert_noop!(
MultiSig::reject_as_identity(bob.origin(), ms_address.clone(), proposal_id),
Error::ProposalAlreadyRejected
);
});
}

#[test]
fn proposal_owner_rejection_denied() {
ExtBuilder::default().build().execute_with(|| {
let eve: User = User::new(AccountKeyring::Eve);
let bob: User = User::new(AccountKeyring::Bob);
let dave: User = User::new(AccountKeyring::Dave);
let alice: User = User::new(AccountKeyring::Alice);

// Creates a multi-signature
let ms_address =
MultiSig::get_next_multisig_address(AccountKeyring::Alice.to_account_id()).unwrap();
setup_multisig(
alice.origin(),
3,
vec![
Signatory::from(alice.did),
Signatory::from(bob.did),
Signatory::from(dave.did),
Signatory::from(eve.did),
],
);

// Creates a proposal
let call1 = Box::new(RuntimeCall::MultiSig(
multisig::Call::change_sigs_required { sigs_required: 4 },
));
assert_ok!(MultiSig::create_proposal_as_identity(
alice.origin(),
ms_address.clone(),
call1,
None,
true
));
let proposal_id = MultiSig::ms_tx_done(ms_address.clone()) - 1;

// The owner of the proposal shouldn't be able to reject it since bob has already voted
assert_ok!(MultiSig::reject_as_identity(
bob.origin(),
ms_address.clone(),
proposal_id
));
assert_noop!(
MultiSig::reject_as_identity(alice.origin(), ms_address.clone(), proposal_id),
Error::AlreadyVoted
);

// The proposal status must be set to Active
let proposal_details = ProposalDetail::<TestStorage>::get(&ms_address, proposal_id);
assert_eq!(proposal_details.status, ProposalStatus::ActiveOrExpired);
assert_eq!(proposal_details.approvals, 1);
assert_eq!(proposal_details.rejections, 1);
assert_eq!(proposal_details.auto_close, true);
assert_eq!(
Votes::<TestStorage>::get((&ms_address, proposal_id), Signatory::from(alice.did)),
true
);
assert_eq!(
Votes::<TestStorage>::get((&ms_address, proposal_id), Signatory::from(bob.did)),
true
);

// No user should be able to change their vote
assert_noop!(
MultiSig::reject_as_identity(bob.origin(), ms_address.clone(), proposal_id),
Error::AlreadyVoted
);
});
}

fn expired_proposals() {
ExtBuilder::default().build().execute_with(|| {
let alice_did = register_keyring_account(AccountKeyring::Alice).unwrap();
Expand Down

0 comments on commit e3f6819

Please sign in to comment.