refactor: clone and drop executor instance on recursive execution

Signed-off-by: Shunkichi Sato <[email protected]>

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;
 

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")
     }
 }

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(())

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<AccountId, u8> = 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<AccountId, u8> = 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(),
                     }
                 }