test: add tests for LSP25

.gitignore

@@ -142,4 +142,4 @@ benchmark.md
 /userdocs
 
 # test temporary folder
-/.test
+/.test

contracts/LSP25ExecuteRelayCall/ILSP25ExecuteRelayCall.sol

@@ -5,7 +5,7 @@ interface ILSP25ExecuteRelayCall {
     /**
      * @notice Reading the latest nonce of address `from` in the channel ID `channelId`.
      *
-     * @dev Get the nonce for a specific controller `from` address that can be used for signing relay transactions via {executeRelayCall}.
+     * @dev Get the nonce for a specific `from` address that can be used for signing relay transactions via {executeRelayCall}.
      *
      * @param from The address of the signer of the transaction.
      * @param channelId The channel id that the signer wants to use for executing the transaction.

contracts/LSP25ExecuteRelayCall/LSP25Errors.sol

@@ -2,16 +2,10 @@
 pragma solidity ^0.8.4;
 
 /**
- * @dev Reverts when the `signer` address retrieved from the `signature` has an invalid nonce: `invalidNonce`.
+ * @notice Relay call not valid yet.
  *
- * @param signer The address of the signer
- * @param invalidNonce The nonce retrieved for the `signer` address
- * @param signature The signature used to retrieve the `signer` address
- */
-error InvalidRelayNonce(address signer, uint256 invalidNonce, bytes signature);
-
-/**
- * @dev Reverts when the start timestamp provided to {executeRelayCall} function is bigger than the current timestamp.
+ * @dev Reverts when the relay call is cannot yet bet executed.
+ * This mean that the starting timestamp provided to {executeRelayCall} function is bigger than the current timestamp.
  */
 error RelayCallBeforeStartTime();
 

contracts/LSP25ExecuteRelayCall/LSP25MultiChannelNonce.sol

@@ -8,11 +8,7 @@ import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
 import {LSP25_VERSION} from "./LSP25Constants.sol";
 
 // errors
-import {
-    InvalidRelayNonce,
-    RelayCallBeforeStartTime,
-    RelayCallExpired
-} from "./LSP25Errors.sol";
+import {RelayCallBeforeStartTime, RelayCallExpired} from "./LSP25Errors.sol";
 
 /**
  * @title Implementation of the multi channel nonce and the signature verification defined in the LSP25 standard.
@@ -31,29 +27,59 @@ abstract contract LSP25MultiChannelNonce {
     mapping(address => mapping(uint256 => uint256)) internal _nonceStore;
 
     /**
-     * @dev Validate that the `nonce` given for the `signature` signed and the `payload` to execute is valid
-     * and conform to the signature format according to the LSP25 standard.
+     * @dev Read the nonce for a `from` address on a specific `channelId`.
+     * This will return an `idx`, which is the concatenation of two `uint128` as follow:
+     * 1. the `channelId` where the nonce was queried for.
+     * 2. the actual nonce of the given `channelId`.
      *
-     * @param signature A valid signature for a signer, generated according to the signature format specified in the LSP25 standard.
-     * @param nonce The nonce that the signer used to generate the `signature`.
-     * @param validityTimestamps Two `uint128` concatenated together, where the left-most `uint128` represent the timestamp from which the transaction can be executed,
-     * and the right-most `uint128` represents the timestamp after which the transaction expire.
-     * @param callData The abi-encoded function call to execute.
+     * For example, if on `channelId` number `5`, the latest nonce  is `1`, the `idx` returned by this function will be:
+     *
+     * ```
+     * // in decimals = 1701411834604692317316873037158841057281
+     * idx = 0x0000000000000000000000000000000500000000000000000000000000000001
+     * ```
      *
-     * @return recoveredSignerAddress The address of the signer recovered, for which the signature was validated.
+     * This idx can be described as follow:
      *
-     * @custom:warning Be aware that this function can also throw an error if the `callData` was signed incorrectly (not conforming to the signature format defined in the LSP25 standard).
-     * The contract cannot distinguish if the data is signed correctly or not. Instead, it will recover an incorrect signer address from the signature
-     * and throw an {InvalidRelayNonce} error with the incorrect signer address as the first parameter.
+     * ```
+     *             channelId => 5          nonce in this channel => 1
+     *   v------------------------------v-------------------------------v
+     * 0x0000000000000000000000000000000500000000000000000000000000000001
+     * ```
+     *
+     * @param from The address to read the nonce for.
+     * @param channelId The channel in which to extract the nonce.
+     *
+     * @return idx The idx composed of two `uint128`: the channelId + nonce in channel concatenated together in a single `uint256` value.
      */
-    function _validateExecuteRelayCall(
+    function _getNonce(
+        address from,
+        uint128 channelId
+    ) internal view virtual returns (uint256 idx) {
+        uint256 nonceInChannel = _nonceStore[from][channelId];
+        return (uint256(channelId) << 128) | nonceInChannel;
+    }
+
+    /**
+     * @dev Recover the address of the signer that generated a `signature` using the parameters provided `nonce`, `validityTimestamps`, `msgValue` and `callData`.
+     * The address of the signer will be recovered using the LSP25 signature format.
+     *
+     * @param signature A 65 bytes long signature generated according to the signature format specified in the LSP25 standard.
+     * @param nonce The nonce that the signer used to generate the `signature`.
+     * @param validityTimestamps The validity timestamp that the signer used to generate the signature (See {_verifyValidityTimestamps} to learn more).
+     * @param msgValue The amount of native tokens intended to be sent for the relay transaction.
+     * @param callData The calldata to execute as a relay transaction that the signer signed for.
+     *
+     * @return The address that signed, recovered from the `signature`.
+     */
+    function _recoverSignerFromLSP25Signature(
         bytes memory signature,
         uint256 nonce,
         uint256 validityTimestamps,
         uint256 msgValue,
         bytes calldata callData
-    ) internal returns (address recoveredSignerAddress) {
-        bytes memory encodedMessage = abi.encodePacked(
+    ) internal view returns (address) {
+        bytes memory lsp25EncodedMessage = abi.encodePacked(
             LSP25_VERSION,
             block.chainid,
             nonce,
@@ -62,31 +88,36 @@ abstract contract LSP25MultiChannelNonce {
             callData
         );
 
-        recoveredSignerAddress = address(this)
-            .toDataWithIntendedValidatorHash(encodedMessage)
-            .recover(signature);
+        bytes32 eip191Hash = address(this).toDataWithIntendedValidatorHash(
+            lsp25EncodedMessage
+        );
 
-        if (!_isValidNonce(recoveredSignerAddress, nonce)) {
-            revert InvalidRelayNonce(recoveredSignerAddress, nonce, signature);
-        }
+        return eip191Hash.recover(signature);
+    }
 
-        // increase nonce after successful verification
-        _nonceStore[recoveredSignerAddress][nonce >> 128]++;
+    /**
+     * @notice Verifying if the current timestamp is within the date and time range provided by `validityTimestamps`.
+     *
+     * @dev Verify that the validity timestamp provided is within a valid range compared to the current timestamp.
+     *
+     * @param validityTimestamps Two `uint128` concatenated together, where the left-most `uint128` represent the timestamp from which the transaction can be executed,
+     * and the right-most `uint128` represents the timestamp after which the transaction expire.
+     */
+    function _verifyValidityTimestamps(
+        uint256 validityTimestamps
+    ) internal view {
+        if (validityTimestamps == 0) return;
 
-        if (validityTimestamps != 0) {
-            uint128 startingTimestamp = uint128(validityTimestamps >> 128);
-            uint128 endingTimestamp = uint128(validityTimestamps);
+        uint128 startingTimestamp = uint128(validityTimestamps >> 128);
+        uint128 endingTimestamp = uint128(validityTimestamps);
 
-            // solhint-disable not-rely-on-time
-            if (block.timestamp < startingTimestamp) {
-                revert RelayCallBeforeStartTime();
-            }
-            if (block.timestamp > endingTimestamp) {
-                revert RelayCallExpired();
-            }
+        // solhint-disable not-rely-on-time
+        if (block.timestamp < startingTimestamp) {
+            revert RelayCallBeforeStartTime();
+        }
+        if (block.timestamp > endingTimestamp) {
+            revert RelayCallExpired();
         }
-
-        return recoveredSignerAddress;
     }
 
     /**
@@ -109,6 +140,6 @@ abstract contract LSP25MultiChannelNonce {
         // Alternatively:
         // uint256 mask = (1<<128)-1;
         // uint256 mask = 0xffffffffffffffffffffffffffffffff;
-        return (idx & mask) == (_nonceStore[from][idx >> 128]);
+        return (idx & mask) == _nonceStore[from][idx >> 128];
     }
 }

contracts/LSP6KeyManager/LSP6Errors.sol

@@ -59,9 +59,9 @@ error InvalidLSP6Target();
  *
  * @dev Reverts when the `signer` address retrieved from the `signature` has an invalid nonce: `invalidNonce`.
  *
- * @param signer The address of the signer
- * @param invalidNonce The nonce retrieved for the `signer` address
- * @param signature The signature used to retrieve the `signer` address
+ * @param signer The address of the signer.
+ * @param invalidNonce The nonce retrieved for the `signer` address.
+ * @param signature The signature used to retrieve the `signer` address.
  */
 error InvalidRelayNonce(address signer, uint256 invalidNonce, bytes signature);
 
@@ -227,20 +227,6 @@ error CannotSendValueToSetData();
  */
 error CallingKeyManagerNotAllowed();
 
-/**
- * @notice Relay call not valid yet.
- *
- * @dev Reverts when the start timestamp provided to {executeRelayCall} function is bigger than the current timestamp.
- */
-error RelayCallBeforeStartTime();
-
-/**
- * @notice The date of the relay call expired.
- *
- * @dev Reverts when the period to execute the relay call has expired.
- */
-error RelayCallExpired();
-
 /**
  * @dev reverts when the address of the Key Manager is being set as extensions for lsp20 functions
  */

contracts/LSP6KeyManager/LSP6KeyManagerCore.sol

@@ -41,6 +41,7 @@ import {
     LSP6BatchExcessiveValueSent,
     LSP6BatchInsufficientValueSent,
     InvalidPayload,
+    InvalidRelayNonce,
     NoPermissionsSet,
     InvalidERC725Function,
     CannotSendValueToSetData
@@ -113,15 +114,22 @@ abstract contract LSP6KeyManagerCore is
     /**
      * @inheritdoc ILSP25
      *
-     * @custom:info A signer can choose its channel number arbitrarily. Channel ID = 0 can be used for sequential nonces (transactions
-     * that are order dependant), any other channel ID for out-of-order execution (= execution in parallel).
+     * @custom:hint A signer can choose its channel number arbitrarily. The recommended practice is to:
+     * - use `channelId == 0` for transactions for which the ordering of execution matters.abi
+     *
+     * _Example: you have two transactions A and B, and transaction A must be executed first and complete successfully before
+     * transaction B should be executed)._
+     *
+     * - use any other `channelId` number for transactions that you want to be order independant (out-of-order execution, execution _"in parallel"_).
+     *
+     * _Example: you have two transactions A and B. You want transaction B to be executed a) without having to wait for transaction A to complete,
+     * or b) regardless if transaction A completed successfully or not.
      */
     function getNonce(
         address from,
         uint128 channelId
     ) public view virtual returns (uint256) {
-        uint256 nonceInChannel = _nonceStore[from][channelId];
-        return (uint256(channelId) << 128) | nonceInChannel;
+        return LSP25MultiChannelNonce._getNonce(from, channelId);
     }
 
     /**
@@ -386,6 +394,20 @@ abstract contract LSP6KeyManagerCore is
         return result;
     }
 
+    /**
+     * @dev Validate that the `nonce` given for the `signature` signed and the `payload` to execute is valid
+     * and conform to the signature format according to the LSP25 standard.
+     *
+     * @param signature A valid signature for a signer, generated according to the signature format specified in the LSP25 standard.
+     * @param nonce The nonce that the signer used to generate the `signature`.
+     * @param validityTimestamps Two `uint128` concatenated together, where the left-most `uint128` represent the timestamp from which the transaction can be executed,
+     * and the right-most `uint128` represents the timestamp after which the transaction expire.
+     * @param payload The abi-encoded function call to execute.
+     *
+     * @custom:warning Be aware that this function can also throw an error if the `callData` was signed incorrectly (not conforming to the signature format defined in the LSP25 standard).
+     * This is because the contract cannot distinguish if the data is signed correctly or not. Instead, it will recover an incorrect signer address from the signature
+     * and throw an {InvalidRelayNonce} error with the incorrect signer address as the first parameter.
+     */
     function _executeRelayCall(
         bytes memory signature,
         uint256 nonce,
@@ -397,13 +419,23 @@ abstract contract LSP6KeyManagerCore is
             revert InvalidPayload(payload);
         }
 
-        address signer = LSP25MultiChannelNonce._validateExecuteRelayCall(
-            signature,
-            nonce,
-            validityTimestamps,
-            msgValue,
-            payload
-        );
+        address signer = LSP25MultiChannelNonce
+            ._recoverSignerFromLSP25Signature(
+                signature,
+                nonce,
+                validityTimestamps,
+                msgValue,
+                payload
+            );
+
+        if (!_isValidNonce(signer, nonce)) {
+            revert InvalidRelayNonce(signer, nonce, signature);
+        }
+
+        // increase nonce after successful verification
+        _nonceStore[signer][nonce >> 128]++;
+
+        LSP25MultiChannelNonce._verifyValidityTimestamps(validityTimestamps);
 
         bool isSetData = false;
         if (

contracts/Mocks/LSP25MultiChannelNonceTester.sol

@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.4;
+
+import {
+    LSP25MultiChannelNonce
+} from "../LSP25ExecuteRelayCall/LSP25MultiChannelNonce.sol";
+
+/**
+ * @dev This contract is used only for testing the internal functions.
+ */
+contract LSP25MultiChannelNonceTester is LSP25MultiChannelNonce {
+    function getNonce(
+        address from,
+        uint128 channelId
+    ) public view returns (uint256 idx) {
+        return _getNonce(from, channelId);
+    }
+
+    function recoverSignerFromLSP25Signature(
+        bytes memory signature,
+        uint256 nonce,
+        uint256 validityTimestamps,
+        uint256 msgValue,
+        bytes calldata callData
+    ) public view returns (address) {
+        return
+            _recoverSignerFromLSP25Signature(
+                signature,
+                nonce,
+                validityTimestamps,
+                msgValue,
+                callData
+            );
+    }
+
+    function verifyValidityTimestamps(uint256 validityTimestamps) public view {
+        return _verifyValidityTimestamps(validityTimestamps);
+    }
+
+    function isValidNonce(
+        address from,
+        uint256 nonce
+    ) public view returns (bool) {
+        return _isValidNonce(from, nonce);
+    }
+}