Skip to content

Commit

Permalink
review: dedup and delegate MultisigRegister validation to execution
Browse files Browse the repository at this point in the history
Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com>
  • Loading branch information
s8sato committed Nov 8, 2024
1 parent ddbbf6f commit e0390e9
Showing 1 changed file with 7 additions and 50 deletions.
57 changes: 7 additions & 50 deletions crates/iroha_executor/src/default/isi/multisig/account.rs
Original file line number Diff line number Diff line change
@@ -1,53 +1,9 @@
//! Validation and execution logic of instructions for multisig accounts

use iroha_executor_data_model::permission::account::CanRegisterAccount;

use super::*;
use crate::permission::domain::is_domain_owner;

impl VisitExecute for MultisigRegister {
fn visit<V: Execute + Visit + ?Sized>(&self, executor: &mut V) {
let registrant = executor.context().authority.clone();
let target_domain = self.account.domain();
let host = executor.host();

let Ok(is_domain_owner) = is_domain_owner(target_domain, &registrant, host) else {
deny!(
executor,
"domain must exist before registering multisig account"
);
};

let has_permission = {
CanRegisterAccount {
domain: target_domain.clone(),
}
.is_owned_by(&registrant, host)
};

// Impose the same restriction as for personal account registrations
// TODO Allow the signatories to register the multisig account? With propose and approve procedures?
if !(is_domain_owner || has_permission) {
deny!(
executor,
"registrant must have sufficient permission to register an account"
);
}

for signatory in self.signatories.keys().cloned() {
if host
.query(FindAccounts)
.filter_with(|account| account.id.eq(signatory))
.execute_single()
.is_err()
{
deny!(
executor,
"signatories must exist before registering multisig account"
);
}
}
}
fn visit<V: Execute + Visit + ?Sized>(&self, _executor: &mut V) {}

fn execute<V: Execute + Visit + ?Sized>(self, executor: &mut V) -> Result<(), ValidationFail> {
let domain_owner = executor
Expand All @@ -58,17 +14,18 @@ impl VisitExecute for MultisigRegister {
.dbg_unwrap()
.owned_by()
.clone();

// Authorize as the domain owner:
// Just having permission to register accounts is insufficient to register multisig roles
executor.context_mut().authority = domain_owner.clone();

let multisig_account = self.account;
let multisig_role = multisig_role_for(&multisig_account);

// The multisig registrant needs to have sufficient permission to register personal accounts
// TODO Loosen to just being one of the signatories? But impose the procedure of propose and approve?
visit_seq!(executor
.visit_register_account(&Register::account(Account::new(multisig_account.clone()))));

// Authorize as the domain owner:
// Just having permission to register accounts is insufficient to register multisig roles
executor.context_mut().authority = domain_owner.clone();

visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account(
multisig_account.clone(),
SIGNATORIES.parse().unwrap(),
Expand Down

0 comments on commit e0390e9

Please sign in to comment.