FIX: fix issue about disableRoles when disconnectSafe

keyper-labs · Jun 27, 2024 · f78d5bc · f78d5bc
1 parent becba4a
commit f78d5bc
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 86 deletions.
diff --git a/src/Helpers.sol b/src/Helpers.sol
@@ -41,7 +41,9 @@ abstract contract Helpers is DenyHelper, SignatureDecoder, ReentrancyGuard {
     /// @return Hash of the domain separator
     function domainSeparator() public view returns (bytes32) {
         return keccak256(
-            abi.encode(Constants.DOMAIN_SEPARATOR_TYPEHASH, getChainId(), address(this))
+            abi.encode(
+                Constants.DOMAIN_SEPARATOR_TYPEHASH, getChainId(), address(this)
+            )
         );
     }
 

diff --git a/src/PalmeraModule.sol b/src/PalmeraModule.sol
@@ -791,10 +791,10 @@ contract PalmeraModule is Auth, Helpers {
         bytes32 org,
         address targetSafe
     ) public view returns (bool hasNotPermission) {
-        hasNotPermission = !isRootSafeOf(caller, getSafeIdBySafe(org, targetSafe))
-            && !isSuperSafe(
-                getSafeIdBySafe(org, caller), getSafeIdBySafe(org, targetSafe)
-            ) && !isSafeLead(getSafeIdBySafe(org, targetSafe), caller);
+        uint256 targetSafeId = getSafeIdBySafe(org, targetSafe);
+        hasNotPermission = !isRootSafeOf(caller, targetSafeId)
+            && !isSuperSafe(getSafeIdBySafe(org, caller), targetSafeId)
+            && !isSafeLead(targetSafeId, caller);
     }
 
     /// @notice check if the Organisation is registered
@@ -1092,28 +1092,13 @@ contract PalmeraModule is Auth, Helpers {
     /// @param user Address of the user to disable roles
     function disableSafeLeadRoles(address user) private {
         RolesAuthority _authority = RolesAuthority(rolesAuthority);
-        if (_authority.doesUserHaveRole(user, uint8(DataTypes.Role.SAFE_LEAD)))
-        {
-            _authority.setUserRole(user, uint8(DataTypes.Role.SAFE_LEAD), false);
-        }
-        if (
-            _authority.doesUserHaveRole(
-                user, uint8(DataTypes.Role.SAFE_LEAD_EXEC_ON_BEHALF_ONLY)
-            )
-        ) {
-            _authority.setUserRole(
-                user, uint8(DataTypes.Role.SAFE_LEAD_EXEC_ON_BEHALF_ONLY), false
-            );
-        }
-        if (
-            _authority.doesUserHaveRole(
-                user, uint8(DataTypes.Role.SAFE_LEAD_MODIFY_OWNERS_ONLY)
-            )
-        ) {
-            _authority.setUserRole(
-                user, uint8(DataTypes.Role.SAFE_LEAD_MODIFY_OWNERS_ONLY), false
-            );
-        }
+        _authority.setUserRole(user, uint8(DataTypes.Role.SAFE_LEAD), false);
+        _authority.setUserRole(
+            user, uint8(DataTypes.Role.SAFE_LEAD_EXEC_ON_BEHALF_ONLY), false
+        );
+        _authority.setUserRole(
+            user, uint8(DataTypes.Role.SAFE_LEAD_MODIFY_OWNERS_ONLY), false
+        );
     }
 
     /// @notice Private method to remove indexId from mapping of indexes into organisations

diff --git a/src/SigningUtils.sol b/src/SigningUtils.sol
@@ -1,7 +1,6 @@
 // SPDX-License-Identifier: LGPL-3.0-only
 pragma solidity 0.8.23;
 
-import {ECDSA} from "@openzeppelin/utils/cryptography/ECDSA.sol";
 import {Enum} from "@safe-contracts/base/Executor.sol";
 
 /// @title SigningUtils
@@ -20,62 +19,4 @@ abstract contract SigningUtils {
         address refundReceiver;
         bytes signatures;
     }
-
-    // /**
-    //  * @dev Given an already https://eips.ethereum.org/EIPS/eip-712#definition-of-hashstruct[hashed struct], this
-    //  * function returns the hash of the fully encoded EIP712 message for this domain.
-    //  *
-    //  * This hash can be used together with {ECDSA-recover} to obtain the signer of a message. For example:
-    //  *
-    //  * ```solidity
-    //  * bytes32 digest = _hashTypedDataV4(keccak256(abi.encode(
-    //  *     keccak256("Mail(address to,string contents)"),
-    //  *     mailTo,
-    //  *     keccak256(bytes(mailContents))
-    //  * )));
-    //  * address signer = ECDSA.recover(digest, signature);
-    //  * ```
-    //  */
-    // function _hashTypedDataV4(bytes32 domainSeparator, bytes32 structHash)
-    //     internal
-    //     view
-    //     virtual
-    //     returns (bytes32)
-    // {
-    //     return ECDSA.toTypedDataHash(domainSeparator, structHash);
-    // }
-
-    // /**
-    //  * @dev Given a transaction, it creates a hash of the transaction that can be signed
-    //  * @param domainSeparatorSafe Hash of the Safe domain separator
-    //  * @param safeTx Safe transaction
-    //  * @return Hash of the transaction
-    //  */
-    // function createDigestExecTx(
-    //     bytes32 domainSeparatorSafe,
-    //     Transaction memory safeTx
-    // ) public view returns (bytes32) {
-    //     bytes32 digest = _hashTypedDataV4(
-    //         domainSeparatorSafe,
-    //         keccak256(
-    //             abi.encode(
-    //                 keccak256(
-    //                     "execTransaction(address to,uint256 value,bytes data,Enum.Operation operation,uint256 safeTxGas,uint256 baseGas,uint256 gasPrice,address gasToken,address refundReceiver,bytes signatures)"
-    //                 ),
-    //                 safeTx.to,
-    //                 safeTx.value,
-    //                 safeTx.data,
-    //                 safeTx.operation,
-    //                 safeTx.safeTxGas,
-    //                 safeTx.baseGas,
-    //                 safeTx.gasPrice,
-    //                 safeTx.gasToken,
-    //                 safeTx.refundReceiver,
-    //                 safeTx.signatures
-    //             )
-    //         )
-    //     );
-
-    //     return digest;
-    // }
 }