perf: use try / catch in LSP20 verification to optimise gas cost (#768)

* perf: use `try / catch` in LSP20 to reduce runtime cost

Co-authored-by: b00ste.lyx <[email protected]>

* perf: use `try / catch` in `_verifyCallResult`

* test: fix revert reason when `bytes4` returnStatus cannot be decoded

* docs: update ABI + internal functions docs

* refactor: replace error parameter to `bytes4`

---------

Co-authored-by: b00ste.lyx <[email protected]>
Co-authored-by: b00ste <[email protected]>

Author: CJ42

contracts/LSP20CallVerification/LSP20CallVerification.sol

@@ -20,48 +20,61 @@ 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 returned status (lsp20VerifyCall selector).
+     * - Returns whether a verification after the execution should happen based on the last byte of the `returnedStatus`.
+     * - 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 returnedStatus) {
+            if (
+                bytes3(returnedStatus) !=
+                bytes3(ILSP20.lsp20VerifyCall.selector)
+            ) {
+                revert LSP20CallVerificationFailed({
+                    postCall: false,
+                    returnedStatus: returnedStatus
+                });
+            }
 
-        if (bytes3(returnedStatus) != bytes3(ILSP20.lsp20VerifyCall.selector)) {
-            revert LSP20CallVerificationFailed(false, returnedData);
+            return returnedStatus[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 returned status (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 +86,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 returnedStatus) {
+            if (returnedStatus != ILSP20.lsp20VerifyCallResult.selector) {
+                revert LSP20CallVerificationFailed({
+                    postCall: true,
+                    returnedStatus: returnedStatus
+                });
+            }
 
-        // 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/LSP20CallVerification/LSP20Errors.sol

@@ -10,9 +10,9 @@ error LSP20CallingVerifierFailed(bool postCall);
 /**
  * @dev reverts when the call to the owner does not return the LSP20 success value
  * @param postCall True if the execution call was done, False otherwise
- * @param returnedData The data returned by the call to the logic verifier
+ * @param returnedStatus The bytes4 decoded data returned by the logic verifier.
  */
-error LSP20CallVerificationFailed(bool postCall, bytes returnedData);
+error LSP20CallVerificationFailed(bool postCall, bytes4 returnedStatus);
 
 /**
  * @dev Reverts when the logic verifier is an Externally Owned Account (EOA) that cannot return the LSP20 success value.

docs/contracts/LSP0ERC725Account/LSP0ERC725Account.md

@@ -1225,8 +1225,6 @@ function _verifyCall(
 ```
 
 Calls [`lsp20VerifyCall`](#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
 
 <br/>
 
@@ -1240,19 +1238,6 @@ function _verifyCallResult(
 ```
 
 Calls [`lsp20VerifyCallResult`](#lsp20verifycallresult) function on the logicVerifier.
-Reverts in case the value returned does not match the success value (lsp20VerifyCallResult selector)
-
-<br/>
-
-### \_validateCall
-
-```solidity
-function _validateCall(
-  bool postCall,
-  bool success,
-  bytes returnedData
-) internal pure;
-```
 
 <br/>
 
@@ -1840,23 +1825,23 @@ Reverts when trying to renounce ownership before the initial confirmation delay.
 
 - Specification details: [**LSP-0-ERC725Account**](https://github.com/lukso-network/lips/tree/main/LSPs/LSP-0-ERC725Account.md#lsp20callverificationfailed)
 - Solidity implementation: [`LSP0ERC725Account.sol`](https://github.com/lukso-network/lsp-smart-contracts/blob/develop/contracts/LSP0ERC725Account/LSP0ERC725Account.sol)
-- Error signature: `LSP20CallVerificationFailed(bool,bytes)`
-- Error hash: `0x00c28d0f`
+- Error signature: `LSP20CallVerificationFailed(bool,bytes4)`
+- Error hash: `0x9d6741e3`
 
 :::
 
 ```solidity
-error LSP20CallVerificationFailed(bool postCall, bytes returnedData);
+error LSP20CallVerificationFailed(bool postCall, bytes4 returnedStatus);
 ```
 
 reverts when the call to the owner does not return the LSP20 success value
 
 #### Parameters
 
-| Name           |  Type   | Description                                          |
-| -------------- | :-----: | ---------------------------------------------------- |
-| `postCall`     | `bool`  | True if the execution call was done, False otherwise |
-| `returnedData` | `bytes` | The data returned by the call to the logic verifier  |
+| Name             |   Type   | Description                                             |
+| ---------------- | :------: | ------------------------------------------------------- |
+| `postCall`       |  `bool`  | True if the execution call was done, False otherwise    |
+| `returnedStatus` | `bytes4` | The bytes4 decoded data returned by the logic verifier. |
 
 <br/>
 

docs/contracts/LSP20CallVerification/LSP20CallVerification.md

@@ -33,8 +33,6 @@ function _verifyCall(
 ```
 
 Calls [`lsp20VerifyCall`](#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
 
 <br/>
 
@@ -48,19 +46,6 @@ function _verifyCallResult(
 ```
 
 Calls [`lsp20VerifyCallResult`](#lsp20verifycallresult) function on the logicVerifier.
-Reverts in case the value returned does not match the success value (lsp20VerifyCallResult selector)
-
-<br/>
-
-### \_validateCall
-
-```solidity
-function _validateCall(
-  bool postCall,
-  bool success,
-  bytes returnedData
-) internal pure;
-```
 
 <br/>
 

docs/contracts/UniversalProfile.md

@@ -1168,8 +1168,6 @@ function _verifyCall(
 ```
 
 Calls [`lsp20VerifyCall`](#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
 
 <br/>
 
@@ -1183,19 +1181,6 @@ function _verifyCallResult(
 ```
 
 Calls [`lsp20VerifyCallResult`](#lsp20verifycallresult) function on the logicVerifier.
-Reverts in case the value returned does not match the success value (lsp20VerifyCallResult selector)
-
-<br/>
-
-### \_validateCall
-
-```solidity
-function _validateCall(
-  bool postCall,
-  bool success,
-  bytes returnedData
-) internal pure;
-```
 
 <br/>
 
@@ -1783,23 +1768,23 @@ Reverts when trying to renounce ownership before the initial confirmation delay.
 
 - Specification details: [**UniversalProfile**](https://github.com/lukso-network/lips/tree/main/LSPs/LSP-3-UniversalProfile-Metadata.md#lsp20callverificationfailed)
 - Solidity implementation: [`UniversalProfile.sol`](https://github.com/lukso-network/lsp-smart-contracts/blob/develop/contracts/UniversalProfile.sol)
-- Error signature: `LSP20CallVerificationFailed(bool,bytes)`
-- Error hash: `0x00c28d0f`
+- Error signature: `LSP20CallVerificationFailed(bool,bytes4)`
+- Error hash: `0x9d6741e3`
 
 :::
 
 ```solidity
-error LSP20CallVerificationFailed(bool postCall, bytes returnedData);
+error LSP20CallVerificationFailed(bool postCall, bytes4 returnedStatus);
 ```
 
 reverts when the call to the owner does not return the LSP20 success value
 
 #### Parameters
 
-| Name           |  Type   | Description                                          |
-| -------------- | :-----: | ---------------------------------------------------- |
-| `postCall`     | `bool`  | True if the execution call was done, False otherwise |
-| `returnedData` | `bytes` | The data returned by the call to the logic verifier  |
+| Name             |   Type   | Description                                             |
+| ---------------- | :------: | ------------------------------------------------------- |
+| `postCall`       |  `bool`  | True if the execution call was done, False otherwise    |
+| `returnedStatus` | `bytes4` | The bytes4 decoded data returned by the logic verifier. |
 
 <br/>
 

tests/LSP20CallVerification/LSP20CallVerification.behaviour.ts

@@ -266,9 +266,9 @@ export const shouldBehaveLikeLSP20 = (buildContext: () => Promise<LSP20TestConte
           const dataKey = ethers.utils.keccak256(ethers.utils.toUtf8Bytes('RandomKey1'));
           const dataValue = ethers.utils.hexlify(ethers.utils.randomBytes(50));
 
-          await expect(context.universalProfile.setData(dataKey, dataValue))
-            .to.be.revertedWithCustomError(context.universalProfile, 'LSP20CallVerificationFailed')
-            .withArgs(false, '0x');
+          await expect(
+            context.universalProfile.setData(dataKey, dataValue),
+          ).to.be.revertedWithoutReason();
         });
       });
 
@@ -335,7 +335,7 @@ export const shouldBehaveLikeLSP20 = (buildContext: () => Promise<LSP20TestConte
 
           await expect(
             context.universalProfile.setData(dataKey, dataValue),
-          ).to.be.revertedWithCustomError(context.universalProfile, 'LSP20CallVerificationFailed');
+          ).to.be.revertedWithoutReason();
         });
       });
 
@@ -368,7 +368,7 @@ export const shouldBehaveLikeLSP20 = (buildContext: () => Promise<LSP20TestConte
 
           await expect(context.universalProfile.setData(dataKey, dataValue))
             .to.be.revertedWithCustomError(context.universalProfile, 'LSP20CallVerificationFailed')
-            .withArgs(false, '0xaabbccdd' + '0'.repeat(56));
+            .withArgs(false, '0xaabbccdd');
         });
       });