From e71898ce58f3131c1e32c66cfbea91a4651d9041 Mon Sep 17 00:00:00 2001
From: Dmitry Murzin <diralik@yandex.ru>
Date: Mon, 18 Nov 2024 17:17:08 +0300
Subject: [PATCH] Review fixes

Signed-off-by: Dmitry Murzin <diralik@yandex.ru>
---
 crates/iroha_executor/src/default/mod.rs | 24 ++++++++++++------------
 crates/iroha_executor/src/permission.rs  | 21 ++++++---------------
 2 files changed, 18 insertions(+), 27 deletions(-)

diff --git a/crates/iroha_executor/src/default/mod.rs b/crates/iroha_executor/src/default/mod.rs
index b1276cf644..26d354a3f5 100644
--- a/crates/iroha_executor/src/default/mod.rs
+++ b/crates/iroha_executor/src/default/mod.rs
@@ -202,11 +202,11 @@ pub mod domain {
                     .is_owned_by(&executor.context().authority, executor.host())
             }
         {
-            revoke_permissions(executor, |permission| {
+            let err = revoke_permissions(executor, |permission| {
                 is_permission_domain_associated(permission, domain_id)
             });
-            if executor.verdict().is_err() {
-                return;
+            if let Err(err) = err {
+                deny!(executor, err);
             }
 
             execute!(executor, isi);
@@ -424,11 +424,11 @@ pub mod account {
                     .is_owned_by(&executor.context().authority, executor.host())
             }
         {
-            revoke_permissions(executor, |permission| {
+            let err = revoke_permissions(executor, |permission| {
                 is_permission_account_associated(permission, account_id)
             });
-            if executor.verdict().is_err() {
-                return;
+            if let Err(err) = err {
+                deny!(executor, err);
             }
 
             execute!(executor, isi);
@@ -603,11 +603,11 @@ pub mod asset_definition {
                     .is_owned_by(&executor.context().authority, executor.host())
             }
         {
-            revoke_permissions(executor, |permission| {
+            let err = revoke_permissions(executor, |permission| {
                 is_permission_asset_definition_associated(permission, asset_definition_id)
             });
-            if executor.verdict().is_err() {
-                return;
+            if let Err(err) = err {
+                deny!(executor, err);
             }
 
             execute!(executor, isi);
@@ -1368,11 +1368,11 @@ pub mod trigger {
                     .is_owned_by(&executor.context().authority, executor.host())
             }
         {
-            revoke_permissions(executor, |permission| {
+            let err = revoke_permissions(executor, |permission| {
                 is_permission_trigger_associated(permission, trigger_id)
             });
-            if executor.verdict().is_err() {
-                return;
+            if let Err(err) = err {
+                deny!(executor, err);
             }
 
             execute!(executor, isi);
diff --git a/crates/iroha_executor/src/permission.rs b/crates/iroha_executor/src/permission.rs
index 08ec2482a6..e5ef2a51b6 100644
--- a/crates/iroha_executor/src/permission.rs
+++ b/crates/iroha_executor/src/permission.rs
@@ -5,7 +5,6 @@ use alloc::{borrow::ToOwned as _, vec::Vec};
 use iroha_executor_data_model::permission::Permission;
 
 use crate::{
-    deny,
     prelude::Context,
     smart_contract::{
         data_model::{executor::Result, permission::Permission as PermissionObject, prelude::*},
@@ -1089,34 +1088,26 @@ pub(crate) fn roles_permissions(host: &Iroha) -> impl Iterator<Item = (RoleId, P
 
 /// Revoked all permissions satisfied given [condition].
 ///
-/// Note: you must manually check after this function:
-/// `if executor.verdict().is_err() { return; }`
+/// Note: you must manually call `deny!` if this function returns error.
 pub(crate) fn revoke_permissions<V: Execute + ?Sized>(
     executor: &mut V,
     condition: impl Fn(&PermissionObject) -> bool,
-) {
-    let mut err = None;
+) -> Result<(), ValidationFail> {
     for (owner_id, permission) in accounts_permissions(executor.host()) {
         if condition(&permission) {
             let isi = Revoke::account_permission(permission, owner_id.clone());
 
-            if let Err(error) = executor.host().submit(&isi) {
-                err = Some(error);
-                break;
-            }
+            executor.host().submit(&isi)?;
         }
     }
-    if let Some(err) = err {
-        deny!(executor, err);
-    }
 
     for (role_id, permission) in roles_permissions(executor.host()) {
         if condition(&permission) {
             let isi = Revoke::role_permission(permission, role_id.clone());
 
-            if let Err(err) = executor.host().submit(&isi) {
-                deny!(executor, err);
-            }
+            executor.host().submit(&isi)?;
         }
     }
+
+    Ok(())
 }