From 5b0dbeb38c1c3ce0a8bd6a517acc142fbe0c4fdd Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Sun, 3 Nov 2024 17:30:23 +0900 Subject: [PATCH] refactor: clone and drop executor instance on recursive execution Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- crates/iroha_executor/src/lib.rs | 2 +- wasm/libs/default_executor/src/lib.rs | 12 ++----- .../default_executor/src/multisig/account.rs | 29 ++++++--------- .../src/multisig/transaction.rs | 36 ++++++++----------- 4 files changed, 29 insertions(+), 50 deletions(-) diff --git a/crates/iroha_executor/src/lib.rs b/crates/iroha_executor/src/lib.rs index 3df84bdb351..5108038d8b5 100644 --- a/crates/iroha_executor/src/lib.rs +++ b/crates/iroha_executor/src/lib.rs @@ -265,7 +265,7 @@ pub trait Execute { /// Represents the current state of the world fn context(&self) -> &prelude::Context; - /// Mutable context for e.g. switching to another authority after validation before execution. + /// Mutable context for e.g. switching to another authority on recursive execution. /// Note that mutations are persistent to the instance unless reset fn context_mut(&mut self) -> &mut prelude::Context; diff --git a/wasm/libs/default_executor/src/lib.rs b/wasm/libs/default_executor/src/lib.rs index 0403dd0162f..26015af53d6 100644 --- a/wasm/libs/default_executor/src/lib.rs +++ b/wasm/libs/default_executor/src/lib.rs @@ -56,27 +56,19 @@ fn visit_custom(executor: &mut Executor, isi: &CustomInstruction) { trait VisitExecute: Instruction { fn visit_execute(self, executor: &mut Executor) { - let init_authority = executor.context().authority.clone(); self.visit(executor); if executor.verdict().is_ok() { - if let Err(err) = self.execute(executor, &init_authority) { + if let Err(err) = self.execute(executor) { executor.deny(err); } } - // reset authority per instruction - // TODO seek a more proper way - executor.context_mut().authority = init_authority; } fn visit(&self, _executor: &mut Executor) { unimplemented!("should be overridden unless `Self::visit_execute` is overridden") } - fn execute( - self, - _executor: &mut Executor, - _init_authority: &AccountId, - ) -> Result<(), ValidationFail> { + fn execute(self, _executor: &Executor) -> Result<(), ValidationFail> { unimplemented!("should be overridden unless `Self::visit_execute` is overridden") } } diff --git a/wasm/libs/default_executor/src/multisig/account.rs b/wasm/libs/default_executor/src/multisig/account.rs index 58a0caa6551..23dd4f332d6 100644 --- a/wasm/libs/default_executor/src/multisig/account.rs +++ b/wasm/libs/default_executor/src/multisig/account.rs @@ -17,7 +17,7 @@ impl VisitExecute for MultisigRegister { ) } - let Ok(domain_found) = host + let Ok(_domain_found) = host .query(FindDomains) .filter_with(|domain| domain.id.eq(target_domain.clone())) .execute_single() @@ -40,23 +40,16 @@ impl VisitExecute for MultisigRegister { ); }; } - - // Pass validation and elevate to the domain owner authority - executor.context_mut().authority = domain_found.owned_by().clone(); } - fn execute( - self, - executor: &mut Executor, - _init_authority: &AccountId, - ) -> Result<(), ValidationFail> { + fn execute(self, executor: &Executor) -> Result<(), ValidationFail> { let host = executor.host(); - let domain_owner = executor.context().authority.clone(); + let registrant = executor.context().authority.clone(); let multisig_account = self.account; let multisig_role = multisig_role_for(&multisig_account); host.submit(&Register::account(Account::new(multisig_account.clone()))) - .dbg_expect("domain owner should successfully register a multisig account"); + .dbg_expect("registrant should successfully register a multisig account"); host.submit(&SetKeyValue::account( multisig_account.clone(), @@ -80,22 +73,22 @@ impl VisitExecute for MultisigRegister { .dbg_unwrap(); host.submit(&Register::role( - // Temporarily grant a multisig role to the domain owner to delegate the role to the signatories - Role::new(multisig_role.clone(), domain_owner.clone()), + // Temporarily grant a multisig role to the registrant to delegate the role to the signatories + Role::new(multisig_role.clone(), registrant.clone()), )) - .dbg_expect("domain owner should successfully register a multisig role"); + .dbg_expect("registrant should successfully register a multisig role"); for signatory in self.signatories.keys().cloned() { host.submit(&Grant::account_role(multisig_role.clone(), signatory)) .dbg_expect( - "domain owner should successfully grant the multisig role to signatories", + "registrant should successfully grant the multisig role to signatories", ); } - // FIXME No roles to revoke found, which should have been granted to the domain owner - // host.submit(&Revoke::account_role(multisig_role, domain_owner)) + // FIXME No roles to revoke found, which should have been granted to the registrant + // host.submit(&Revoke::account_role(multisig_role, registrant)) // .dbg_expect( - // "domain owner should successfully revoke the multisig role from the domain owner", + // "registrant should successfully revoke the multisig role from the registrant", // ); Ok(()) diff --git a/wasm/libs/default_executor/src/multisig/transaction.rs b/wasm/libs/default_executor/src/multisig/transaction.rs index fe9f476e38a..759b0357283 100644 --- a/wasm/libs/default_executor/src/multisig/transaction.rs +++ b/wasm/libs/default_executor/src/multisig/transaction.rs @@ -42,17 +42,11 @@ impl VisitExecute for MultisigPropose { )) else { deny!(executor, "multisig proposal duplicates") }; - - // Pass validation and elevate to the multisig account authority - executor.context_mut().authority = target_account; } - fn execute( - self, - executor: &mut Executor, - init_authority: &AccountId, - ) -> Result<(), ValidationFail> { + fn execute(self, executor: &Executor) -> Result<(), ValidationFail> { let host = executor.host().clone(); + let proposer = executor.context().authority.clone(); let target_account = self.account; let instructions_hash = HashOf::new(&self.instructions); let signatories: BTreeMap = host @@ -80,7 +74,9 @@ impl VisitExecute for MultisigPropose { MultisigPropose::new(signatory, [approve_me.into()].to_vec()) }; - propose_to_approve_me.visit_execute(executor); + let mut executor = executor.clone(); + executor.context_mut().authority = target_account.clone(); + propose_to_approve_me.execute(&executor)?; } } @@ -91,7 +87,7 @@ impl VisitExecute for MultisigPropose { .as_millis() .try_into() .dbg_unwrap(); - let approvals = BTreeSet::from([init_authority.clone()]); + let approvals = BTreeSet::from([proposer]); host.submit(&SetKeyValue::account( target_account.clone(), @@ -108,7 +104,7 @@ impl VisitExecute for MultisigPropose { .dbg_unwrap(); host.submit(&SetKeyValue::account( - target_account.clone(), + target_account, approvals_key(&instructions_hash).clone(), Json::new(&approvals), )) @@ -140,17 +136,11 @@ impl VisitExecute for MultisigApprove { )) else { deny!(executor, "no proposals to approve") }; - - // Pass validation and elevate to the multisig account authority - executor.context_mut().authority = target_account; } - fn execute( - self, - executor: &mut Executor, - init_authority: &AccountId, - ) -> Result<(), ValidationFail> { + fn execute(self, executor: &Executor) -> Result<(), ValidationFail> { let host = executor.host().clone(); + let approver = executor.context().authority.clone(); let target_account = self.account; let instructions_hash = self.instructions_hash; let signatories: BTreeMap = host @@ -202,7 +192,7 @@ impl VisitExecute for MultisigApprove { .try_into_any() .dbg_unwrap(); - approvals.insert(init_authority.clone()); + approvals.insert(approver); host.submit(&SetKeyValue::account( target_account.clone(), @@ -252,7 +242,11 @@ impl VisitExecute for MultisigApprove { // Execute instructions proposal which collected enough approvals for isi in instructions { match isi { - InstructionBox::Custom(instruction) => visit_custom(executor, &instruction), + InstructionBox::Custom(instruction) => { + let mut executor = executor.clone(); + executor.context_mut().authority = target_account.clone(); + visit_custom(&mut executor, &instruction) + } builtin => host.submit(&builtin).dbg_unwrap(), } }