perf: optimize runtime gas cost for LSP6 + LSP20

contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol

@@ -821,6 +821,7 @@ abstract contract LSP0ERC725AccountCore is
         bytes memory dataValue
     ) internal virtual override {
         ERC725YCore._store[dataKey] = dataValue;
+
         emit DataChanged(
             dataKey,
             dataValue.length <= 256

contracts/LSP20CallVerification/LSP20CallVerification.sol

@@ -20,48 +20,58 @@ import {
 abstract contract LSP20CallVerification {
     /**
      * @dev Calls {lsp20VerifyCall} function on the logicVerifier.
-     * Reverts in case the value returned does not match the success value (lsp20VerifyCall selector)
-     * Returns whether a verification after the execution should happen based on the last byte of the returnedStatus
+     *
+     * @custom:info
+     * - Reverts in case the value returned does not match the magic value (lsp20VerifyCall selector).
+     * - Returns whether a verification after the execution should happen based on the last byte of the magicValue.
+     * - Reverts with no reason if the  data returned by `ILSP20(logicVerifier).lsp20VerifyCall(...)` cannot be decoded (_e.g:_ any other data type besides `bytes4`).
+     * See this link for more info: https://forum.soliditylang.org/t/call-for-feedback-the-future-of-try-catch-in-solidity/1497.
      */
     function _verifyCall(
         address logicVerifier
     ) internal virtual returns (bool verifyAfter) {
-        if (logicVerifier.code.length == 0)
+        if (logicVerifier.code.length == 0) {
             revert LSP20EOACannotVerifyCall(logicVerifier);
+        }
 
-        (bool success, bytes memory returnedData) = logicVerifier.call(
-            abi.encodeWithSelector(
-                ILSP20.lsp20VerifyCall.selector,
+        // Reverts with no reason if the returned data type is not a `bytes4` value
+        try
+            ILSP20(logicVerifier).lsp20VerifyCall(
                 msg.sender,
                 address(this),
                 msg.sender,
                 msg.value,
                 msg.data
             )
-        );
-
-        _validateCall(false, success, returnedData);
-
-        bytes4 returnedStatus = abi.decode(returnedData, (bytes4));
+        returns (bytes4 magicValue) {
+            if (bytes3(magicValue) != bytes3(ILSP20.lsp20VerifyCall.selector)) {
+                revert LSP20CallVerificationFailed(
+                    false,
+                    abi.encode(magicValue)
+                );
+            }
 
-        if (bytes3(returnedStatus) != bytes3(ILSP20.lsp20VerifyCall.selector)) {
-            revert LSP20CallVerificationFailed(false, returnedData);
+            return magicValue[3] == 0x01;
+        } catch (bytes memory errorData) {
+            _revertWithLSP20DefaultError(false, errorData);
         }
-
-        return returnedStatus[3] == 0x01;
     }
 
     /**
      * @dev Calls {lsp20VerifyCallResult} function on the logicVerifier.
-     * Reverts in case the value returned does not match the success value (lsp20VerifyCallResult selector)
+     *
+     * @custom:info
+     * - Reverts in case the value returned does not match the magic value (lsp20VerifyCallResult selector).
+     * - Reverts with no reason if the  data returned by `ILSP20(logicVerifier).lsp20VerifyCallResult(...)` cannot be decoded (_e.g:_ any other data type besides `bytes4`).
+     * See this link for more info: https://forum.soliditylang.org/t/call-for-feedback-the-future-of-try-catch-in-solidity/1497.
      */
     function _verifyCallResult(
         address logicVerifier,
         bytes memory callResult
     ) internal virtual {
-        (bool success, bytes memory returnedData) = logicVerifier.call(
-            abi.encodeWithSelector(
-                ILSP20.lsp20VerifyCallResult.selector,
+        // Reverts with no reason if the returned data type is not a `bytes4` value
+        try
+            ILSP20(logicVerifier).lsp20VerifyCallResult(
                 keccak256(
                     abi.encodePacked(
                         msg.sender,
@@ -73,37 +83,18 @@ abstract contract LSP20CallVerification {
                 ),
                 callResult
             )
-        );
-
-        _validateCall(true, success, returnedData);
-
-        if (
-            abi.decode(returnedData, (bytes4)) !=
-            ILSP20.lsp20VerifyCallResult.selector
-        )
-            revert LSP20CallVerificationFailed({
-                postCall: true,
-                returnedData: returnedData
-            });
-    }
-
-    function _validateCall(
-        bool postCall,
-        bool success,
-        bytes memory returnedData
-    ) internal pure virtual {
-        if (!success) _revertWithLSP20DefaultError(postCall, returnedData);
+        returns (bytes4 magicValue) {
+            if (magicValue != ILSP20.lsp20VerifyCallResult.selector) {
+                revert LSP20CallVerificationFailed(
+                    true,
+                    abi.encode(magicValue)
+                );
+            }
 
-        // check if the returned data contains at least 32 bytes, potentially an abi encoded bytes4 value
-        // check if the returned data has in the first 32 bytes an abi encoded bytes4 value
-        if (
-            returnedData.length < 32 ||
-            bytes28(bytes32(returnedData) << 32) != bytes28(0)
-        )
-            revert LSP20CallVerificationFailed({
-                postCall: postCall,
-                returnedData: returnedData
-            });
+            return;
+        } catch (bytes memory errorData) {
+            _revertWithLSP20DefaultError(true, errorData);
+        }
     }
 
     function _revertWithLSP20DefaultError(

contracts/LSP4DigitalAssetMetadata/LSP4DigitalAssetMetadata.sol

@@ -65,6 +65,7 @@ abstract contract LSP4DigitalAssetMetadata is ERC725Y {
             revert LSP4TokenSymbolNotEditable();
         } else {
             _store[dataKey] = dataValue;
+
             emit DataChanged(
                 dataKey,
                 dataValue.length <= 256

contracts/LSP4DigitalAssetMetadata/LSP4DigitalAssetMetadataInitAbstract.sol

@@ -69,6 +69,7 @@ abstract contract LSP4DigitalAssetMetadataInitAbstract is ERC725YInitAbstract {
             revert LSP4TokenSymbolNotEditable();
         } else {
             _store[dataKey] = dataValue;
+
             emit DataChanged(
                 dataKey,
                 dataValue.length <= 256

contracts/LSP6KeyManager/LSP6KeyManagerCore.sol

@@ -10,6 +10,9 @@ import {
     IERC725Y
 } from "@erc725/smart-contracts/contracts/interfaces/IERC725Y.sol";
 import {ILSP6KeyManager as ILSP6} from "./ILSP6KeyManager.sol";
+import {
+    ILSP14Ownable2Step as ILSP14
+} from "../LSP14Ownable2Step/ILSP14Ownable2Step.sol";
 import {
     ILSP20CallVerifier as ILSP20
 } from "../LSP20CallVerification/ILSP20CallVerifier.sol";
@@ -18,7 +21,6 @@ import {
 } from "../LSP25ExecuteRelayCall/ILSP25ExecuteRelayCall.sol";
 
 // modules
-import {ILSP14Ownable2Step} from "../LSP14Ownable2Step/ILSP14Ownable2Step.sol";
 import {ERC725Y} from "@erc725/smart-contracts/contracts/ERC725Y.sol";
 import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol";
 import {LSP6SetDataModule} from "./LSP6Modules/LSP6SetDataModule.sol";
@@ -32,7 +34,6 @@ import {
 } from "../LSP25ExecuteRelayCall/LSP25MultiChannelNonce.sol";
 
 // libraries
-import {BytesLib} from "solidity-bytes-utils/contracts/BytesLib.sol";
 import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
 import {Address} from "@openzeppelin/contracts/utils/Address.sol";
 import {LSP6Utils} from "./LSP6Utils.sol";
@@ -91,7 +92,6 @@ abstract contract LSP6KeyManagerCore is
 {
     using LSP6Utils for *;
     using ECDSA for *;
-    using BytesLib for bytes;
 
     address internal _target;
 
@@ -324,48 +324,117 @@ abstract contract LSP6KeyManagerCore is
         uint256 msgValue,
         bytes calldata callData
     ) external virtual override returns (bytes4) {
-        bool isSetData = bytes4(callData) == IERC725Y.setData.selector ||
-            bytes4(callData) == IERC725Y.setDataBatch.selector;
+        bytes32 permissions = ERC725Y(targetContract).getPermissionsFor(caller);
+        if (permissions == bytes32(0)) revert NoPermissionsSet(caller);
+
+        bool reentrancyStatus = _reentrancyStatus[targetContract];
+        if (reentrancyStatus) {
+            _requirePermissions(caller, permissions, _PERMISSION_REENTRANCY);
+        }
+
+        bytes4 erc725Function = bytes4(callData);
+
+        bytes4 lsp20MagicValue = _lsp20VerifyPermissions(
+            targetContract,
+            caller,
+            permissions,
+            erc725Function,
+            callData,
+            reentrancyStatus
+        );
 
         // If target is invoking the verification, emit the event and change the reentrancy guard
         if (msg.sender == targetContract) {
-            bool reentrancyStatus = _nonReentrantBefore(
+            if (!reentrancyStatus) {
+                if (
+                    bytes4(callData) != IERC725Y.setData.selector &&
+                    bytes4(callData) != IERC725Y.setDataBatch.selector
+                ) {
+                    _reentrancyStatus[targetContract] = true;
+                }
+            }
+
+            emit PermissionsVerified(caller, msgValue, erc725Function);
+        }
+
+        return lsp20MagicValue;
+    }
+
+    function _lsp20VerifyPermissions(
+        address targetContract,
+        address caller,
+        bytes32 permissions,
+        bytes4 erc725Function,
+        bytes calldata data,
+        bool reentrancyStatus
+    ) internal view returns (bytes4) {
+        if (erc725Function == IERC725Y.setData.selector) {
+            (bytes32 inputKey, bytes memory inputValue) = abi.decode(
+                data[4:],
+                (bytes32, bytes)
+            );
+
+            LSP6SetDataModule._verifyCanSetData(
+                targetContract,
+                caller,
+                permissions,
+                inputKey,
+                inputValue
+            );
+
+            return _LSP20_VERIFY_CALL_SUCCESS_VALUE_WITHOUT_POST_VERIFICATION;
+        } else if (erc725Function == IERC725Y.setDataBatch.selector) {
+            (bytes32[] memory inputKeys, bytes[] memory inputValues) = abi
+                .decode(data[4:], (bytes32[], bytes[]));
+
+            LSP6SetDataModule._verifyCanSetData(
                 targetContract,
-                isSetData,
-                caller
+                caller,
+                permissions,
+                inputKeys,
+                inputValues
             );
 
-            _verifyPermissions(targetContract, caller, false, callData);
+            return _LSP20_VERIFY_CALL_SUCCESS_VALUE_WITHOUT_POST_VERIFICATION;
+        } else if (erc725Function == IERC725X.execute.selector) {
+            (
+                uint256 operationType,
+                address to,
+                uint256 value,
+                bytes memory payload
+            ) = abi.decode(data[4:], (uint256, address, uint256, bytes));
 
-            emit PermissionsVerified(caller, msgValue, bytes4(callData));
+            LSP6ExecuteModule._verifyCanExecute(
+                targetContract,
+                caller,
+                permissions,
+                operationType,
+                to,
+                value,
+                payload
+            );
 
-            // if it's a setData call, do not invoke the `lsp20VerifyCallResult(..)` function
             return
-                isSetData || reentrancyStatus
+                reentrancyStatus
                     ? _LSP20_VERIFY_CALL_SUCCESS_VALUE_WITHOUT_POST_VERIFICATION
                     : _LSP20_VERIFY_CALL_SUCCESS_VALUE_WITH_POST_VERIFICATION;
-        }
-        /// @dev If a different address is invoking the verification,
-        /// do not change the state or emit the event to allow read-only verification
-        else {
-            bool reentrancyStatus = _reentrancyStatus[targetContract];
-
-            if (reentrancyStatus) {
-                _requirePermissions(
-                    caller,
-                    ERC725Y(targetContract).getPermissionsFor(caller),
-                    _PERMISSION_REENTRANCY
-                );
-            }
-
-            _verifyPermissions(targetContract, caller, false, callData);
+        } else if (
+            erc725Function == ILSP14.transferOwnership.selector ||
+            erc725Function == ILSP14.acceptOwnership.selector ||
+            erc725Function == ILSP14.renounceOwnership.selector
+        ) {
+            LSP6OwnershipModule._verifyOwnershipPermissions(
+                caller,
+                permissions
+            );
 
-            // if it's a setData call, do not invoke the `lsp20VerifyCallResult(..)` function
             return
-                isSetData || reentrancyStatus
+                reentrancyStatus
                     ? _LSP20_VERIFY_CALL_SUCCESS_VALUE_WITHOUT_POST_VERIFICATION
                     : _LSP20_VERIFY_CALL_SUCCESS_VALUE_WITH_POST_VERIFICATION;
         }
+
+        revert InvalidERC725Function(erc725Function);
     }
 
     /**
@@ -376,10 +445,9 @@ abstract contract LSP6KeyManagerCore is
         bytes memory /* callResult */
     ) external virtual override returns (bytes4) {
         // If it's the target calling, set back the reentrancy guard
-        // to false, if not return the success value
-        if (msg.sender == _target) {
-            _nonReentrantAfter(msg.sender);
-        }
+        // to false, if not return the magic value
+        _nonReentrantAfter(msg.sender);
+
         return _LSP20_VERIFY_CALL_RESULT_SUCCESS_VALUE;
     }
 
@@ -584,9 +652,9 @@ abstract contract LSP6KeyManagerCore is
                 data
             );
         } else if (
-            erc725Function == ILSP14Ownable2Step.transferOwnership.selector ||
-            erc725Function == ILSP14Ownable2Step.acceptOwnership.selector ||
-            erc725Function == ILSP14Ownable2Step.renounceOwnership.selector
+            erc725Function == ILSP14.transferOwnership.selector ||
+            erc725Function == ILSP14.acceptOwnership.selector ||
+            erc725Function == ILSP14.renounceOwnership.selector
         ) {
             LSP6OwnershipModule._verifyOwnershipPermissions(from, permissions);
         } else {
@@ -595,8 +663,8 @@ abstract contract LSP6KeyManagerCore is
     }
 
     /**
-     * @dev Update the status from `_NON_ENTERED` to `_ENTERED` and checks if
-     * the status is `_ENTERED` in order to revert the call unless the caller has the REENTRANCY permission
+     * @dev Update the status from `true` to `false` and checks if
+     * the status is `true` in order to revert the call unless the caller has the REENTRANCY permission
      * Used in the beginning of the `nonReentrant` modifier, before the method execution starts.
      */
     function _nonReentrantBefore(
@@ -625,8 +693,6 @@ abstract contract LSP6KeyManagerCore is
      * Used in the end of the `nonReentrant` modifier after the method execution is terminated
      */
     function _nonReentrantAfter(address targetContract) internal virtual {
-        // By storing the original value once again, a refund is triggered (see
-        // https://eips.ethereum.org/EIPS/eip-2200)
         _reentrancyStatus[targetContract] = false;
     }