chore: [C4] Add first batch of QA fixes

contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol

@@ -46,7 +46,7 @@ import {
     _TYPEID_LSP0_OwnershipTransferStarted,
     _TYPEID_LSP0_OwnershipTransferred_SenderNotification,
     _TYPEID_LSP0_OwnershipTransferred_RecipientNotification
-} from "../LSP0ERC725Account/LSP0Constants.sol";
+} from "./LSP0Constants.sol";
 import {
     _INTERFACEID_LSP1,
     _LSP1_UNIVERSAL_RECEIVER_DELEGATE_PREFIX,
@@ -63,7 +63,8 @@ import {
 
 // errors
 import {
-    ERC725Y_DataKeysValuesLengthMismatch
+    ERC725Y_DataKeysValuesLengthMismatch,
+    ERC725Y_DataKeysValuesEmptyArray
 } from "@erc725/smart-contracts/contracts/errors.sol";
 import {
     NoExtensionFoundForFunctionSelector
@@ -203,16 +204,16 @@ abstract contract LSP0ERC725AccountCore is
             emit ValueReceived(msg.sender, msg.value);
         }
 
-        address _owner = owner();
+        address accountOwner = owner();
 
         // If the caller is the owner perform execute directly
-        if (msg.sender == _owner) {
+        if (msg.sender == accountOwner) {
             return ERC725XCore._execute(operationType, target, value, data);
         }
 
         // If the caller is not the owner, call {lsp20VerifyCall} on the owner
         // Depending on the magicValue returned, a second call is done after execution
-        bool verifyAfter = LSP20CallVerification._verifyCall(_owner);
+        bool verifyAfter = LSP20CallVerification._verifyCall(accountOwner);
 
         // Perform the execution
         bytes memory result = ERC725XCore._execute(
@@ -224,7 +225,10 @@ abstract contract LSP0ERC725AccountCore is
 
         // if verifyAfter is true, Call {lsp20VerifyCallResult} on the owner
         if (verifyAfter) {
-            LSP20CallVerification._verifyCallResult(_owner, abi.encode(result));
+            LSP20CallVerification._verifyCallResult(
+                accountOwner,
+                abi.encode(result)
+            );
         }
 
         return result;
@@ -255,10 +259,10 @@ abstract contract LSP0ERC725AccountCore is
             emit ValueReceived(msg.sender, msg.value);
         }
 
-        address _owner = owner();
+        address accountOwner = owner();
 
         // If the caller is the owner perform execute directly
-        if (msg.sender == _owner) {
+        if (msg.sender == accountOwner) {
             return
                 ERC725XCore._executeBatch(
                     operationsType,
@@ -270,7 +274,7 @@ abstract contract LSP0ERC725AccountCore is
 
         // If the caller is not the owner, call {lsp20VerifyCall} on the owner
         // Depending on the magicValue returned, a second call is done after execution
-        bool verifyAfter = LSP20CallVerification._verifyCall(_owner);
+        bool verifyAfter = LSP20CallVerification._verifyCall(accountOwner);
 
         // Perform the execution
         bytes[] memory results = ERC725XCore._executeBatch(
@@ -283,7 +287,7 @@ abstract contract LSP0ERC725AccountCore is
         // if verifyAfter is true, Call {lsp20VerifyCallResult} on the owner
         if (verifyAfter) {
             LSP20CallVerification._verifyCallResult(
-                _owner,
+                accountOwner,
                 abi.encode(results)
             );
         }
@@ -308,23 +312,23 @@ abstract contract LSP0ERC725AccountCore is
             emit ValueReceived(msg.sender, msg.value);
         }
 
-        address _owner = owner();
+        address accountOwner = owner();
 
         // If the caller is the owner perform setData directly
-        if (msg.sender == _owner) {
+        if (msg.sender == accountOwner) {
             return _setData(dataKey, dataValue);
         }
 
         // If the caller is not the owner, call {lsp20VerifyCall} on the owner
         // Depending on the magicValue returned, a second call is done after setting data
-        bool verifyAfter = _verifyCall(_owner);
+        bool verifyAfter = _verifyCall(accountOwner);
 
         _setData(dataKey, dataValue);
 
         // If verifyAfter is true, Call {lsp20VerifyCallResult} on the owner
         // The setData function does not return, second parameter of {_verifyCallResult} will be empty
         if (verifyAfter) {
-            _verifyCallResult(_owner, "");
+            _verifyCallResult(accountOwner, "");
         }
     }
 
@@ -349,10 +353,14 @@ abstract contract LSP0ERC725AccountCore is
             revert ERC725Y_DataKeysValuesLengthMismatch();
         }
 
-        address _owner = owner();
+        if (dataKeys.length == 0) {
+            revert ERC725Y_DataKeysValuesEmptyArray();
+        }
+
+        address accountOwner = owner();
 
         // If the caller is the owner perform setData directly
-        if (msg.sender == _owner) {
+        if (msg.sender == accountOwner) {
             for (uint256 i = 0; i < dataKeys.length; ) {
                 _setData(dataKeys[i], dataValues[i]);
 
@@ -366,7 +374,7 @@ abstract contract LSP0ERC725AccountCore is
 
         // If the caller is not the owner, call {lsp20VerifyCall} on the owner
         // Depending on the magicValue returned, a second call is done after setting data
-        bool verifyAfter = _verifyCall(_owner);
+        bool verifyAfter = _verifyCall(accountOwner);
 
         for (uint256 i = 0; i < dataKeys.length; ) {
             _setData(dataKeys[i], dataValues[i]);
@@ -379,7 +387,7 @@ abstract contract LSP0ERC725AccountCore is
         // If verifyAfter is true, Call {lsp20VerifyCallResult} on the owner
         // The setData function does not return, second parameter of {_verifyCallResult} will be empty
         if (verifyAfter) {
-            _verifyCallResult(_owner, "");
+            _verifyCallResult(accountOwner, "");
         }
     }
 
@@ -584,24 +592,24 @@ abstract contract LSP0ERC725AccountCore is
      *
      * @custom:requirements Can be only called by the {owner} or by an authorised address that pass the verification check performed on the owner.
      *
-     * @custom:danger Leaves the contract without an owner. Once ownership of the contract has been renounced, any functions that are restricted to be called by the owner will be permanently inaccessible, making these functions not callable anymore and unusable.
+     * @custom:danger Leaves the contract without an owner. Once ownership of the contract has been renounced, any functions that are restricted to be called by the owner or an address allowed by the owner will be permanently inaccessible, making these functions not callable anymore and unusable.
      *
      */
     function renounceOwnership()
         public
         virtual
         override(LSP14Ownable2Step, OwnableUnset)
     {
-        address _owner = owner();
+        address accountOwner = owner();
 
         // If the caller is the owner perform renounceOwnership directly
-        if (msg.sender == _owner) {
+        if (msg.sender == accountOwner) {
             return LSP14Ownable2Step._renounceOwnership();
         }
 
         // If the caller is not the owner, call {lsp20VerifyCall} on the owner
         // Depending on the magicValue returned, a second call is done after transferring ownership
-        bool verifyAfter = _verifyCall(_owner);
+        bool verifyAfter = _verifyCall(accountOwner);
 
         address previousOwner = owner();
         LSP14Ownable2Step._renounceOwnership();
@@ -616,7 +624,7 @@ abstract contract LSP0ERC725AccountCore is
         // If verifyAfter is true, Call {lsp20VerifyCallResult} on the owner
         // The transferOwnership function does not return, second parameter of {_verifyCallResult} will be empty
         if (verifyAfter) {
-            _verifyCallResult(_owner, "");
+            _verifyCallResult(accountOwner, "");
         }
     }
 

contracts/LSP14Ownable2Step/ILSP14Ownable2Step.sol

@@ -59,7 +59,7 @@ interface ILSP14Ownable2Step {
      * @dev Transfer ownership of the contract from the current {owner()} to the {pendingOwner()}.
      *
      * Once this function is called:
-     * - The current {owner()} will loose access to the functions restricted to the {owner()} only.
+     * - The current {owner()} will lose access to the functions restricted to the {owner()} only.
      * - The {pendingOwner()} will gain access to the functions restricted to the {owner()} only.
      *
      * @notice `msg.sender` is accepting ownership of contract: `address(this)`.

contracts/LSP20CallVerification/LSP20CallVerification.sol

@@ -39,7 +39,7 @@ abstract contract LSP20CallVerification {
         if (bytes3(magicValue) != bytes3(ILSP20.lsp20VerifyCall.selector))
             revert LSP20InvalidMagicValue(false, returnedData);
 
-        return magicValue[3] == 0x01 ? true : false;
+        return bytes1(magicValue[3]) == 0x01;
     }
 
     /**

contracts/LSP6KeyManager/LSP6KeyManagerCore.sol

@@ -305,13 +305,8 @@ abstract contract LSP6KeyManagerCore is
         uint256 msgValue,
         bytes calldata data
     ) external virtual returns (bytes4) {
-        bool isSetData = false;
-        if (
-            bytes4(data) == IERC725Y.setData.selector ||
-            bytes4(data) == IERC725Y.setDataBatch.selector
-        ) {
-            isSetData = true;
-        }
+        bool isSetData = bytes4(data) == IERC725Y.setData.selector ||
+            bytes4(data) == IERC725Y.setDataBatch.selector;
 
         // If target is invoking the verification, emit the event and change the reentrancy guard
         if (msg.sender == _target) {
@@ -372,13 +367,8 @@ abstract contract LSP6KeyManagerCore is
             revert InvalidPayload(payload);
         }
 
-        bool isSetData = false;
-        if (
-            bytes4(payload) == IERC725Y.setData.selector ||
-            bytes4(payload) == IERC725Y.setDataBatch.selector
-        ) {
-            isSetData = true;
-        }
+        bool isSetData = bytes4(payload) == IERC725Y.setData.selector ||
+            bytes4(payload) == IERC725Y.setDataBatch.selector;
 
         bool isReentrantCall = _nonReentrantBefore(isSetData, msg.sender);
 
@@ -437,13 +427,8 @@ abstract contract LSP6KeyManagerCore is
 
         LSP25MultiChannelNonce._verifyValidityTimestamps(validityTimestamps);
 
-        bool isSetData = false;
-        if (
-            bytes4(payload) == IERC725Y.setData.selector ||
-            bytes4(payload) == IERC725Y.setDataBatch.selector
-        ) {
-            isSetData = true;
-        }
+        bool isSetData = bytes4(payload) == IERC725Y.setData.selector ||
+            bytes4(payload) == IERC725Y.setDataBatch.selector;
 
         bool isReentrantCall = _nonReentrantBefore(isSetData, signer);
 

contracts/LSP7DigitalAsset/LSP7DigitalAssetCore.sol

@@ -219,38 +219,38 @@ abstract contract LSP7DigitalAssetCore is ILSP7DigitalAsset {
      * It has been added in the LSP7 contract implementation so that it can be used as a prevention mechanism
      * against the double spending allowance vulnerability.
      *
-     * @notice Decrease the allowance of `operator` by -`substractedAmount`
+     * @notice Decrease the allowance of `operator` by -`subtractedAmount`
      *
      * @dev Atomically decreases the allowance granted to `operator` by the caller.
      * This is an alternative approach to {authorizeOperator} that can be used as a mitigation
      * for the double spending allowance problem.
      *
      * @custom:events
      *  - {AuthorizedOperator} event indicating the updated allowance after decreasing it.
-     *  - {RevokeOperator} event if `substractedAmount` is the full allowance,
+     *  - {RevokeOperator} event if `subtractedAmount` is the full allowance,
      *    indicating `operator` does not have any alauthorizedAmountForlowance left for `msg.sender`.
      *
      * @param operator the operator to decrease allowance for `msg.sender`
-     * @param substractedAmount the amount to decrease by in the operator's allowance.
+     * @param subtractedAmount the amount to decrease by in the operator's allowance.
      *
      * @custom:requirements
      *  - `operator` cannot be the zero address.
-     *  - `operator` must have allowance for the caller of at least `substractedAmount`.
+     *  - `operator` must have allowance for the caller of at least `subtractedAmount`.
      */
     function decreaseAllowance(
         address operator,
-        uint256 substractedAmount
+        uint256 subtractedAmount
     ) public virtual {
         uint256 currentAllowance = authorizedAmountFor(operator, msg.sender);
-        if (currentAllowance < substractedAmount) {
+        if (currentAllowance < subtractedAmount) {
             revert LSP7DecreasedAllowanceBelowZero();
         }
 
         unchecked {
             _updateOperator(
                 msg.sender,
                 operator,
-                currentAllowance - substractedAmount
+                currentAllowance - subtractedAmount
             );
         }
     }

tests/UniversalProfile.behaviour.ts

@@ -133,6 +133,15 @@ export const shouldBehaveLikeLSP3 = (
       '0xdaea594e385fc724449e3118b2db7e86dfba1826',
     ];
 
+    it('should fail when setting empty data keys', async () => {
+      const keys = [];
+      const values = [];
+
+      await expect(
+        context.universalProfile.setDataBatch(keys, values),
+      ).to.be.revertedWithCustomError(context.universalProfile, 'ERC725Y_DataKeysValuesEmptyArray');
+    });
+
     it('should set the 3 x keys for a basic UP setup => `LSP3Profile`, `LSP12IssuedAssets[]` and `LSP1UniversalReceiverDelegate`', async () => {
       const keys = [
         ERC725YDataKeys.LSP3.LSP3Profile,