Add virtual (#644)

* chore: add missing virtual keyword in LSP6 * chore: add missing virtual keyword in LSP7 * chore: add missing virtual keyword in LSP8 * chore: add missing virtual keyword in LSP9 * chore: add missing virtual keyword in LSP20 * chore: add missing virtual keyword in LSP0 * refactor: add virtual to target() in LSP6 core * chore: rename _revert in LSP20CallVerification * refactor: add virtual to all LSP6 _isAllowed checks
lukso-network · Aug 3, 2023 · e0c7922 · e0c7922
1 parent 8f0cfb2
commit e0c7922
Show file tree

Hide file tree

Showing 15 changed files with 84 additions and 68 deletions.
diff --git a/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol b/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol
@@ -170,7 +170,7 @@ abstract contract LSP0ERC725AccountCore is
      */
     function batchCalls(
         bytes[] calldata data
-    ) public returns (bytes[] memory results) {
+    ) public virtual returns (bytes[] memory results) {
         results = new bytes[](data.length);
         for (uint256 i; i < data.length; ) {
             (bool success, bytes memory result) = address(this).delegatecall(

diff --git a/contracts/LSP20CallVerification/LSP20CallVerification.sol b/contracts/LSP20CallVerification/LSP20CallVerification.sol
@@ -70,8 +70,8 @@ abstract contract LSP20CallVerification {
         bool postCall,
         bool success,
         bytes memory returnedData
-    ) internal pure {
-        if (!success) _revert(postCall, returnedData);
+    ) internal pure virtual {
+        if (!success) _revertWithLSP20DefaultError(postCall, returnedData);
 
         // 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
@@ -81,7 +81,10 @@ abstract contract LSP20CallVerification {
         ) revert LSP20InvalidMagicValue(postCall, returnedData);
     }
 
-    function _revert(bool postCall, bytes memory returnedData) internal pure {
+    function _revertWithLSP20DefaultError(
+        bool postCall,
+        bytes memory returnedData
+    ) internal pure virtual {
         // Look for revert reason and bubble it up if present
         if (returnedData.length > 0) {
             // The easiest way to bubble the revert reason is using memory via assembly

diff --git a/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol b/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
@@ -82,7 +82,7 @@ abstract contract LSP6KeyManagerCore is
 
     mapping(address => mapping(uint256 => uint256)) internal _nonceStore;
 
-    function target() public view returns (address) {
+    function target() public view virtual returns (address) {
         return _target;
     }
 
@@ -116,7 +116,7 @@ abstract contract LSP6KeyManagerCore is
     function isValidSignature(
         bytes32 dataHash,
         bytes memory signature
-    ) public view returns (bytes4 magicValue) {
+    ) public view virtual returns (bytes4 magicValue) {
         // if isValidSignature fail, the error is catched in returnedError
         (address recoveredAddress, ECDSA.RecoverError returnedError) = ECDSA
             .tryRecover(dataHash, signature);
@@ -250,7 +250,7 @@ abstract contract LSP6KeyManagerCore is
         address caller,
         uint256 msgValue,
         bytes calldata data
-    ) external returns (bytes4) {
+    ) external virtual returns (bytes4) {
         bool isSetData = false;
         if (
             bytes4(data) == IERC725Y.setData.selector ||
@@ -301,7 +301,7 @@ abstract contract LSP6KeyManagerCore is
     function lsp20VerifyCallResult(
         bytes32 /*callHash*/,
         bytes memory /*result*/
-    ) external returns (bytes4) {
+    ) external virtual returns (bytes4) {
         // If it's the target calling, set back the reentrancy guard
         // to false, if not return the magic value
         if (msg.sender == _target) {

diff --git a/contracts/LSP6KeyManager/LSP6Modules/LSP6ExecuteModule.sol b/contracts/LSP6KeyManager/LSP6Modules/LSP6ExecuteModule.sol
@@ -364,7 +364,7 @@ abstract contract LSP6ExecuteModule {
     function _isAllowedAddress(
         bytes memory allowedCall,
         address to
-    ) internal pure returns (bool) {
+    ) internal pure virtual returns (bool) {
         // <offset> = 4 bytes x 8 bits = 32 bits
         //
         // <offset>v----------------address---------------v
@@ -380,7 +380,7 @@ abstract contract LSP6ExecuteModule {
     function _isAllowedStandard(
         bytes memory allowedCall,
         address to
-    ) internal view returns (bool) {
+    ) internal view virtual returns (bool) {
         // <offset> = 24 bytes x 8 bits = 192 bits
         //
         //                                                 standard
@@ -397,7 +397,7 @@ abstract contract LSP6ExecuteModule {
     function _isAllowedFunction(
         bytes memory allowedCall,
         bytes4 requiredFunction
-    ) internal pure returns (bool) {
+    ) internal pure virtual returns (bool) {
         // <offset> = 28 bytes x 8 bits = 224 bits
         //
         //                                                         function
@@ -416,7 +416,7 @@ abstract contract LSP6ExecuteModule {
     function _isAllowedCallType(
         bytes memory allowedCall,
         bytes4 requiredCallTypes
-    ) internal pure returns (bool) {
+    ) internal pure virtual returns (bool) {
         // extract callType
         //
         // <offset> = 0

diff --git a/contracts/LSP6KeyManager/LSP6Modules/LSP6SetDataModule.sol b/contracts/LSP6KeyManager/LSP6Modules/LSP6SetDataModule.sol
@@ -438,7 +438,7 @@ abstract contract LSP6SetDataModule {
         bytes32 dataKey,
         bytes memory dataValue,
         bool hasBothAddControllerAndEditPermissions
-    ) internal view returns (bytes32) {
+    ) internal view virtual returns (bytes32) {
         if (!LSP6Utils.isCompactBytesArrayOfAllowedERC725YDataKeys(dataValue)) {
             revert InvalidEncodedAllowedERC725YDataKeys(
                 dataValue,

diff --git a/contracts/LSP7DigitalAsset/extensions/LSP7Burnable.sol b/contracts/LSP7DigitalAsset/extensions/LSP7Burnable.sol
@@ -14,7 +14,11 @@ abstract contract LSP7Burnable is LSP7DigitalAsset {
      *
      * See internal _burn function for more details
      */
-    function burn(address from, uint256 amount, bytes memory data) public {
+    function burn(
+        address from,
+        uint256 amount,
+        bytes memory data
+    ) public virtual {
         _burn(from, amount, data);
     }
 }
diff --git a/contracts/LSP7DigitalAsset/extensions/LSP7BurnableInitAbstract.sol b/contracts/LSP7DigitalAsset/extensions/LSP7BurnableInitAbstract.sol
@@ -16,7 +16,11 @@ abstract contract LSP7BurnableInitAbstract is LSP7DigitalAssetInitAbstract {
      *
      * See internal _burn function for more details
      */
-    function burn(address from, uint256 amount, bytes memory data) public {
+    function burn(
+        address from,
+        uint256 amount,
+        bytes memory data
+    ) public virtual {
         _burn(from, amount, data);
     }
 }
diff --git a/contracts/LSP7DigitalAsset/presets/LSP7CompatibleERC20Mintable.sol b/contracts/LSP7DigitalAsset/presets/LSP7CompatibleERC20Mintable.sol
@@ -15,7 +15,7 @@ contract LSP7CompatibleERC20Mintable is LSP7CompatibleERC20 {
         uint256 amount,
         bool allowNonLSP1Recipient,
         bytes memory data
-    ) public onlyOwner {
+    ) public virtual onlyOwner {
         _mint(to, amount, allowNonLSP1Recipient, data);
     }
 }
diff --git a/contracts/LSP7DigitalAsset/presets/LSP7CompatibleERC20MintableInitAbstract.sol b/contracts/LSP7DigitalAsset/presets/LSP7CompatibleERC20MintableInitAbstract.sol
@@ -25,7 +25,7 @@ contract LSP7CompatibleERC20MintableInitAbstract is
         uint256 amount,
         bool allowNonLSP1Recipient,
         bytes memory data
-    ) public onlyOwner {
+    ) public virtual onlyOwner {
         _mint(to, amount, allowNonLSP1Recipient, data);
     }
 }
diff --git a/contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8Burnable.sol b/contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8Burnable.sol
@@ -13,7 +13,7 @@ import {LSP8NotTokenOperator} from "../LSP8Errors.sol";
  * their own tokens and those that they have an allowance for as an operator.
  */
 abstract contract LSP8Burnable is LSP8IdentifiableDigitalAssetCore {
-    function burn(bytes32 tokenId, bytes memory data) public {
+    function burn(bytes32 tokenId, bytes memory data) public virtual {
         if (!_isOperatorOrOwner(msg.sender, tokenId)) {
             revert LSP8NotTokenOperator(tokenId, msg.sender);
         }

diff --git a/contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8CompatibleERC721.sol b/contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8CompatibleERC721.sol
@@ -310,31 +310,31 @@ abstract contract LSP8CompatibleERC721 is
         uint256 tokenId,
         bytes memory data
     ) private returns (bool) {
-        if (to.code.length > 0) {
-            try
-                IERC721Receiver(to).onERC721Received(
-                    msg.sender,
-                    from,
-                    tokenId,
-                    data
-                )
-            returns (bytes4 retval) {
-                return retval == IERC721Receiver.onERC721Received.selector;
-            } catch (bytes memory reason) {
-                if (reason.length == 0) {
-                    revert(
-                        "LSP8CompatibleERC721: transfer to non ERC721Receiver implementer"
-                    );
-                } else {
-                    // solhint-disable no-inline-assembly
-                    /// @solidity memory-safe-assembly
-                    assembly {
-                        revert(add(32, reason), mload(reason))
-                    }
+        if (to.code.length == 0) {
+            return true;
+        }
+
+        try
+            IERC721Receiver(to).onERC721Received(
+                msg.sender,
+                from,
+                tokenId,
+                data
+            )
+        returns (bytes4 retval) {
+            return retval == IERC721Receiver.onERC721Received.selector;
+        } catch (bytes memory reason) {
+            if (reason.length == 0) {
+                revert(
+                    "LSP8CompatibleERC721: transfer to non ERC721Receiver implementer"
+                );
+            } else {
+                // solhint-disable no-inline-assembly
+                /// @solidity memory-safe-assembly
+                assembly {
+                    revert(add(32, reason), mload(reason))
                 }
             }
-        } else {
-            return true;
         }
     }
 

diff --git a/contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8CompatibleERC721InitAbstract.sol b/contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8CompatibleERC721InitAbstract.sol
@@ -310,31 +310,31 @@ abstract contract LSP8CompatibleERC721InitAbstract is
         uint256 tokenId,
         bytes memory data
     ) private returns (bool) {
-        if (to.code.length > 0) {
-            try
-                IERC721Receiver(to).onERC721Received(
-                    msg.sender,
-                    from,
-                    tokenId,
-                    data
-                )
-            returns (bytes4 retval) {
-                return retval == IERC721Receiver.onERC721Received.selector;
-            } catch (bytes memory reason) {
-                if (reason.length == 0) {
-                    revert(
-                        "LSP8CompatibleERC721: transfer to non ERC721Receiver implementer"
-                    );
-                } else {
-                    // solhint-disable no-inline-assembly
-                    /// @solidity memory-safe-assembly
-                    assembly {
-                        revert(add(32, reason), mload(reason))
-                    }
+        if (to.code.length == 0) {
+            return true;
+        }
+
+        try
+            IERC721Receiver(to).onERC721Received(
+                msg.sender,
+                from,
+                tokenId,
+                data
+            )
+        returns (bytes4 retval) {
+            return retval == IERC721Receiver.onERC721Received.selector;
+        } catch (bytes memory reason) {
+            if (reason.length == 0) {
+                revert(
+                    "LSP8CompatibleERC721: transfer to non ERC721Receiver implementer"
+                );
+            } else {
+                // solhint-disable no-inline-assembly
+                /// @solidity memory-safe-assembly
+                assembly {
+                    revert(add(32, reason), mload(reason))
                 }
             }
-        } else {
-            return true;
         }
     }
 

diff --git a/contracts/LSP8IdentifiableDigitalAsset/presets/LSP8CompatibleERC721Mintable.sol b/contracts/LSP8IdentifiableDigitalAsset/presets/LSP8CompatibleERC721Mintable.sol
@@ -15,7 +15,7 @@ contract LSP8CompatibleERC721Mintable is LSP8CompatibleERC721 {
         bytes32 tokenId,
         bool allowNonLSP1Recipient,
         bytes memory data
-    ) public onlyOwner {
+    ) public virtual onlyOwner {
         _mint(to, tokenId, allowNonLSP1Recipient, data);
     }
 }
diff --git a/contracts/LSP8IdentifiableDigitalAsset/presets/LSP8CompatibleERC721MintableInitAbstract.sol b/contracts/LSP8IdentifiableDigitalAsset/presets/LSP8CompatibleERC721MintableInitAbstract.sol
@@ -25,7 +25,7 @@ contract LSP8CompatibleERC721MintableInitAbstract is
         bytes32 tokenId,
         bool allowNonLSP1Recipient,
         bytes memory data
-    ) public onlyOwner {
+    ) public virtual onlyOwner {
         _mint(to, tokenId, allowNonLSP1Recipient, data);
     }
 }
diff --git a/contracts/LSP9Vault/LSP9VaultCore.sol b/contracts/LSP9Vault/LSP9VaultCore.sol
@@ -233,7 +233,7 @@ contract LSP9VaultCore is
      */
     function batchCalls(
         bytes[] calldata data
-    ) public returns (bytes[] memory results) {
+    ) public virtual returns (bytes[] memory results) {
         results = new bytes[](data.length);
         for (uint256 i; i < data.length; ) {
             (bool success, bytes memory result) = address(this).delegatecall(
@@ -366,7 +366,12 @@ contract LSP9VaultCore is
     /**
      * @dev Modifier restricting the call to the owner of the contract and the UniversalReceiverDelegate
      */
-    function _validateAndIdentifyCaller() internal view returns (bool isURD) {
+    function _validateAndIdentifyCaller()
+        internal
+        view
+        virtual
+        returns (bool isURD)
+    {
         if (msg.sender != owner()) {
             require(
                 msg.sender == _reentrantDelegate,