From 3c98d624204e8497ed8185c7c71334fc95d1a504 Mon Sep 17 00:00:00 2001
From: CJ42 <cavallerajean@gmail.com>
Date: Fri, 18 Aug 2023 17:59:55 +0100
Subject: [PATCH] refactor: adjust + re-order checks for main controller

---
 .../LSP6Modules/LSP6ExecuteModule.sol         | 26 +++++++++----------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/contracts/LSP6KeyManager/LSP6Modules/LSP6ExecuteModule.sol b/contracts/LSP6KeyManager/LSP6Modules/LSP6ExecuteModule.sol
index 51bab1576..e191590df 100644
--- a/contracts/LSP6KeyManager/LSP6Modules/LSP6ExecuteModule.sol
+++ b/contracts/LSP6KeyManager/LSP6Modules/LSP6ExecuteModule.sol
@@ -189,7 +189,7 @@ abstract contract LSP6ExecuteModule {
         uint256 value,
         bytes memory data
     ) internal view virtual {
-        bool isTransferringValue = value != 0;
+        bool isValueTransfer = value != 0;
 
         bool hasSuperTransferValue = permissions.hasPermission(
             _PERMISSION_SUPER_TRANSFERVALUE
@@ -199,7 +199,7 @@ abstract contract LSP6ExecuteModule {
 
         bool hasSuperCall = permissions.hasPermission(_PERMISSION_SUPER_CALL);
 
-        if (isTransferringValue && !hasSuperTransferValue) {
+        if (isValueTransfer && !hasSuperTransferValue) {
             _requirePermissions(
                 controller,
                 permissions,
@@ -207,9 +207,12 @@ abstract contract LSP6ExecuteModule {
             );
         }
 
+        // Skip if both SUPER permissions are present
+        if (hasSuperCall && hasSuperTransferValue) return;
+
         // CHECK if we are doing an empty call, as the receive() or fallback() function
         // of the controlledContract could run some code.
-        if (!hasSuperCall && isEmptyCall && !isTransferringValue) {
+        if (isEmptyCall && !isValueTransfer && !hasSuperCall) {
             _requirePermissions(controller, permissions, _PERMISSION_CALL);
         }
 
@@ -218,13 +221,10 @@ abstract contract LSP6ExecuteModule {
         }
 
         // Skip if caller has SUPER permissions for external calls, with or without calldata (empty calls)
-        if (hasSuperCall && !isTransferringValue) return;
+        if (!isValueTransfer && hasSuperCall) return;
 
         // Skip if caller has SUPER permission for value transfers
-        if (hasSuperTransferValue && isEmptyCall && isTransferringValue) return;
-
-        // Skip if both SUPER permissions are present
-        if (hasSuperCall && hasSuperTransferValue) return;
+        if (isEmptyCall && isValueTransfer && hasSuperTransferValue) return;
 
         _verifyAllowedCall(
             controlledContract,
@@ -244,11 +244,6 @@ abstract contract LSP6ExecuteModule {
         uint256 value,
         bytes memory data
     ) internal view virtual {
-        bool isEmptyCall = data.length == 0;
-
-        // CHECK if there is at least a 4 bytes function selector
-        bytes4 selector = data.length >= 4 ? bytes4(data) : bytes4(0);
-
         // CHECK for ALLOWED CALLS
         bytes memory allowedCalls = ERC725Y(controlledContract)
             .getAllowedCallsFor(controllerAddress);
@@ -257,6 +252,11 @@ abstract contract LSP6ExecuteModule {
             revert NoCallsAllowed(controllerAddress);
         }
 
+        bool isEmptyCall = data.length == 0;
+
+        // CHECK if there is at least a 4 bytes function selector
+        bytes4 selector = data.length >= 4 ? bytes4(data) : bytes4(0);
+
         bytes4 requiredCallTypes = _extractCallType(
             operationType,
             value,