Payments hashing on contract. Includes chainid

0xsequence · Jun 24, 2024 · 0e5a15c · 0e5a15c
1 parent 8339116
commit 0e5a15c
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 26 deletions.
diff --git a/src/payments/IPayments.sol b/src/payments/IPayments.sol
@@ -52,11 +52,17 @@ interface IPaymentsFunctions {
     function isValidSignature(PaymentDetails calldata paymentDetails, bytes calldata signature)
         external
         view
-        returns (bool);
+        returns (bool isValid);
+
+    /**
+     * Returns the hash of the payment details.
+     * @param paymentDetails The payment details.
+     * @return paymentHash The hash of the payment details for signing.
+     */
+    function hashPaymentDetails(PaymentDetails calldata paymentDetails) external view returns (bytes32 paymentHash);
 }
 
 interface IPaymentsSignals {
-
     /// @notice Emitted when a payment is already accepted. This prevents double spending.
     error PaymentAlreadyAccepted();
 

diff --git a/src/payments/Payments.sol b/src/payments/Payments.sol
@@ -72,9 +72,17 @@ contract Payments is Ownable, IPayments, IERC165 {
         view
         returns (bool)
     {
-        // Check if signature is valid
-        bytes32 messageHash = keccak256(
+        bytes32 messageHash = hashPaymentDetails(paymentDetails);
+        address sigSigner = messageHash.recoverCalldata(signature);
+        return sigSigner == signer;
+    }
+
+    /// @inheritdoc IPaymentsFunctions
+    /// @dev This hash includes the chain ID.
+    function hashPaymentDetails(PaymentDetails calldata paymentDetails) public view returns (bytes32) {
+        return keccak256(
             abi.encode(
+                block.chainid,
                 paymentDetails.purchaseId,
                 paymentDetails.productRecipient,
                 paymentDetails.tokenType,
@@ -86,9 +94,6 @@ contract Payments is Ownable, IPayments, IERC165 {
                 paymentDetails.additionalData
             )
         );
-        //FIXME Check this
-        address sigSigner = messageHash.recoverCalldata(signature);
-        return sigSigner == signer;
     }
 
     /**

diff --git a/test/payments/Payments.t.sol b/test/payments/Payments.t.sol
@@ -98,7 +98,7 @@ contract PaymentsTest is Test, IPaymentsSignals {
         IGenericToken(tokenAddr).approve(caller, address(payments), tokenId, amount);
 
         // Sign it
-        bytes32 messageHash = _hashPaymentDetails(details);
+        bytes32 messageHash = payments.hashPaymentDetails(details);
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPk, messageHash);
         bytes memory sig = abi.encodePacked(r, s, v);
 
@@ -151,7 +151,7 @@ contract PaymentsTest is Test, IPaymentsSignals {
         IGenericToken(tokenAddr).approve(caller, address(payments), tokenId, amount * 2);
 
         // Sign it
-        bytes32 messageHash = _hashPaymentDetails(details);
+        bytes32 messageHash = payments.hashPaymentDetails(details);
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPk, messageHash);
         bytes memory sig = abi.encodePacked(r, s, v);
 
@@ -224,7 +224,7 @@ contract PaymentsTest is Test, IPaymentsSignals {
         IGenericToken(tokenAddr).approve(caller, address(payments), tokenId, amount);
 
         // Sign it
-        bytes32 messageHash = _hashPaymentDetails(details);
+        bytes32 messageHash = payments.hashPaymentDetails(details);
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPk, messageHash);
         bytes memory sig = abi.encodePacked(r, s, v);
 
@@ -234,22 +234,6 @@ contract PaymentsTest is Test, IPaymentsSignals {
         payments.makePayment(details, sig);
     }
 
-    function _hashPaymentDetails(IPaymentsFunctions.PaymentDetails memory details) internal pure returns (bytes32) {
-        return keccak256(
-            abi.encode(
-                details.purchaseId,
-                details.productRecipient,
-                details.tokenType,
-                details.tokenAddress,
-                details.tokenId,
-                details.paymentRecipients,
-                details.expiration,
-                details.productId,
-                details.additionalData
-            )
-        );
-    }
-
     // Update signer
 
     function testUpdateSignerSuccess(address newSigner) public {