Skip to content

Commit

Permalink
fix: prevent a double grant to the registrant who is also a signatory
Browse files Browse the repository at this point in the history
Signed-off-by: Shunkichi Sato <[email protected]>
  • Loading branch information
s8sato committed Nov 6, 2024
1 parent e95f743 commit 45b24d9
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 15 deletions.
6 changes: 3 additions & 3 deletions crates/iroha_executor/src/default/custom/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ mod multisig;

pub fn visit_custom_instructions<V: Execute + Visit + ?Sized>(
executor: &mut V,
isi: &CustomInstruction,
instruction: &CustomInstruction,
) {
if let Ok(isi) = MultisigInstructionBox::try_from(isi.payload()) {
return isi.visit_execute(executor);
if let Ok(instruction) = MultisigInstructionBox::try_from(instruction.payload()) {
return instruction.visit_execute(executor);
};

deny!(executor, "unexpected custom instruction");
Expand Down
11 changes: 5 additions & 6 deletions crates/iroha_executor/src/default/custom/multisig/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ impl VisitExecute for MultisigRegister {

fn execute<V: Execute + Visit + ?Sized>(self, executor: &mut V) -> Result<(), ValidationFail> {
let host = executor.host();
let registrant = executor.context().authority.clone();
let multisig_account = self.account;
let multisig_role = multisig_role_for(&multisig_account);

Expand Down Expand Up @@ -63,8 +62,8 @@ impl VisitExecute for MultisigRegister {
.dbg_unwrap();

host.submit(&Register::role(
// Temporarily grant a multisig role to the registrant to delegate the role to the signatories
Role::new(multisig_role.clone(), registrant.clone()),
// No use, but temporarily grant a multisig role to the multisig account due to specifications
Role::new(multisig_role.clone(), multisig_account.clone()),
))
.dbg_expect("registrant should successfully register a multisig role");

Expand All @@ -75,10 +74,10 @@ impl VisitExecute for MultisigRegister {
);
}

// FIXME No roles to revoke found, which should have been granted to the registrant
// host.submit(&Revoke::account_role(multisig_role, registrant))
// FIXME No roles to revoke found, which should have been granted to the multisig account
// host.submit(&Revoke::account_role(multisig_role, multisig_account))
// .dbg_expect(
// "registrant should successfully revoke the multisig role from the registrant",
// "registrant should successfully revoke the multisig role from the multisig account",
// );

Ok(())
Expand Down
16 changes: 10 additions & 6 deletions crates/iroha_executor/src/default/custom/multisig/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,18 +115,22 @@ impl VisitExecute for MultisigApprove {
let multisig_role = multisig_role_for(&target_account);
let instructions_hash = self.instructions_hash;

let Ok(_role_found) = host
if host
.query(FindRolesByAccountId::new(approver))
.filter_with(|role_id| role_id.eq(multisig_role))
.execute_single()
else {
.is_err()
{
deny!(executor, "not qualified to approve multisig");
};

let Ok(_proposal_found) = host.query_single(FindAccountMetadata::new(
target_account.clone(),
approvals_key(&instructions_hash),
)) else {
if host
.query_single(FindAccountMetadata::new(
target_account.clone(),
approvals_key(&instructions_hash),
))
.is_err()
{
deny!(executor, "no proposals to approve")
};
}
Expand Down

0 comments on commit 45b24d9

Please sign in to comment.