Fix 1271 contract signatures

- Closes #2535 and #2526

Author: Uxio0

safe_transaction_service/safe_messages/serializers.py

@@ -16,7 +16,10 @@
 from safe_transaction_service.utils.serializers import get_safe_owners
 
 from .models import SIGNATURE_LENGTH, SafeMessage, SafeMessageConfirmation
-from .utils import get_hash_for_message, get_safe_message_hash_for_message
+from .utils import (
+    get_message_encoded,
+    get_safe_message_hash_for_message,
+)
 
 
 # Request serializers
@@ -105,12 +108,12 @@ def validate_message(self, value: Union[str, dict[str, Any]]):
     def validate(self, attrs):
         attrs = super().validate(attrs)
         safe_address = self.context["safe_address"]
+        attrs["safe"] = safe_address
         message = attrs["message"]
         signature = attrs["signature"]
-        attrs["safe"] = safe_address
-        message_hash = get_hash_for_message(message)
+        message_encoded = get_message_encoded(message)
         safe_message_hash = get_safe_message_hash_for_message(
-            safe_address, message_hash
+            safe_address, message_encoded
         )
         attrs["message_hash"] = safe_message_hash
 
@@ -120,7 +123,7 @@ def validate(self, attrs):
             )
 
         safe_signatures = SafeSignature.parse_signature(
-            signature, safe_message_hash, safe_hash_preimage=message_hash
+            signature, safe_message_hash, safe_hash_preimage=message_encoded
         )
         owner, signature_type = self.get_valid_owner_from_signatures(
             safe_signatures, safe_address, None
@@ -158,11 +161,11 @@ def validate(self, attrs):
         attrs["safe_message"] = safe_message
         signature: HexStr = attrs["signature"]
         safe_address = safe_message.safe
-        message_hash = get_hash_for_message(safe_message.message)
+        message_encoded = get_message_encoded(safe_message.message)
         safe_message_hash = safe_message.message_hash
 
         safe_signatures = SafeSignature.parse_signature(
-            signature, safe_message_hash, safe_hash_preimage=message_hash
+            signature, safe_message_hash, safe_hash_preimage=message_encoded
         )
         owner, signature_type = self.get_valid_owner_from_signatures(
             safe_signatures, safe_address, safe_message

safe_transaction_service/safe_messages/tests/factories.py

@@ -5,7 +5,7 @@
 from safe_eth.util.util import to_0x_hex_str
 
 from ..models import SafeMessage, SafeMessageConfirmation
-from ..utils import get_hash_for_message, get_safe_message_hash_for_message
+from ..utils import get_message_encoded, get_safe_message_hash_for_message
 
 
 class SafeMessageFactory(DjangoModelFactory):
@@ -22,7 +22,7 @@ class Meta:
     def message_hash(self) -> str:
         return to_0x_hex_str(
             get_safe_message_hash_for_message(
-                self.safe, get_hash_for_message(self.message)
+                self.safe, get_message_encoded(self.message)
             )
         )
 

safe_transaction_service/safe_messages/tests/test_models.py

@@ -8,7 +8,7 @@
 from safe_eth.safe.tests.safe_test_case import SafeTestCaseMixin
 from safe_eth.util.util import to_0x_hex_str
 
-from ..utils import get_hash_for_message, get_safe_message_hash_for_message
+from ..utils import get_message_encoded, get_safe_message_hash_for_message
 from .factories import SafeMessageConfirmationFactory, SafeMessageFactory
 from .mocks import get_eip712_payload_mock
 
@@ -22,15 +22,15 @@ def test_str(self):
         for input, expected in [
             (
                 "TestMessage",
-                "Safe Message 0xb04a24aa07a51d1d8c3913e9493b3b1f88ed6a8a75430a9a8eda3ed3ce1897bc - TestMessage",
+                "Safe Message 0x95a89792bad17115e101b4d8fbdcd517dd13d085e815287d7fe791078182bf9e - TestMessage",
             ),
             (
                 "TestMessageVeryLong",
-                "Safe Message 0xe3db816540ce371e2703b8ec59bdd6fec32e0c6078f2e204a205fd6d81564f28 - TestMessageVery...",
+                "Safe Message 0xb7783fde9060e75b61298c14cbc2a987f0e0800d1bb08b4a2b24ce3544b9b144 - TestMessageVery...",
             ),
             (
                 get_eip712_payload_mock(),
-                "Safe Message 0xbabb22f5c02a24db447b8f0136d6e26bb58cd6d068ebe8ab25c2221cfdf53e18 - {'types': {'EIP...",
+                "Safe Message 0x632db20317ce8e467d17e941d209fc1173ff1cad83b0c072c008385a566ddd80 - {'types': {'EIP...",
             ),
         ]:
             with self.subTest(input=input):
@@ -62,7 +62,7 @@ def test_factory(self):
             message_hash,
             to_0x_hex_str(
                 get_safe_message_hash_for_message(
-                    safe_message.safe, get_hash_for_message(message)
+                    safe_message.safe, get_message_encoded(message)
                 )
             ),
         )

safe_transaction_service/safe_messages/tests/test_views.py

@@ -7,12 +7,11 @@
 
 import eth_abi
 from eth_account import Account
-from eth_account.messages import defunct_hash_message
 from hexbytes import HexBytes
 from rest_framework import status
 from rest_framework.exceptions import ErrorDetail
 from rest_framework.test import APITestCase
-from safe_eth.eth.eip712 import eip712_encode_hash
+from safe_eth.eth.eip712 import eip712_encode
 from safe_eth.safe.safe_signature import SafeSignatureEOA
 from safe_eth.safe.signatures import signature_to_bytes
 from safe_eth.safe.tests.safe_test_case import SafeTestCaseMixin
@@ -28,6 +27,7 @@
 )
 from safe_transaction_service.utils.utils import datetime_to_str
 
+from ..utils import encode_eip191_message, encode_eip712_message
 from .mocks import get_eip712_payload_mock
 
 logger = logging.getLogger(__name__)
@@ -144,12 +144,13 @@ def test_safe_messages_create_view(self, get_owners_mock: MagicMock):
         safe_address = safe.address
         messages = ["Text to sign message", get_eip712_payload_mock()]
         description = "Testing EIP191 message signing"
-        message_hashes = [
-            defunct_hash_message(text=messages[0]),
-            eip712_encode_hash(messages[1]),
+        messages_encoded = [
+            encode_eip191_message(messages[0]),
+            encode_eip712_message(messages[1]),
         ]
         safe_message_hashes = [
-            safe.get_message_hash(message_hash) for message_hash in message_hashes
+            safe.get_message_hash(message_encoded)
+            for message_encoded in messages_encoded
         ]
         signatures = [
             to_0x_hex_str(account.unsafe_sign_hash(safe_message_hash)["signature"])
@@ -158,15 +159,14 @@ def test_safe_messages_create_view(self, get_owners_mock: MagicMock):
 
         sub_tests = ["create_eip191", "create_eip712"]
 
-        for sub_test, message, message_hash, safe_message_hash, signature in zip(
-            sub_tests, messages, message_hashes, safe_message_hashes, signatures
+        for sub_test, message, safe_message_hash, signature in zip(
+            sub_tests, messages, safe_message_hashes, signatures
         ):
             SafeMessage.objects.all().delete()
             get_owners_mock.return_value = []
             with self.subTest(
                 sub_test,
                 message=message,
-                message_hash=message_hash,
                 safe_message_hash=safe_message_hash,
                 signature=signature,
             ):
@@ -289,8 +289,8 @@ def _test_safe_messages_create_using_1271_signature_view(self, safe_deployment_f
         safe_address = safe.address
         message = get_eip712_payload_mock()
         description = "Testing EIP712 message signing"
-        message_hash = eip712_encode_hash(message)
-        safe_owner_message_hash = safe_owner.get_message_hash(message_hash)
+        message_encoded = b"".join(eip712_encode(message))
+        safe_owner_message_hash = safe_owner.get_message_hash(message_encoded)
         safe_owner_signature = account.unsafe_sign_hash(safe_owner_message_hash)[
             "signature"
         ]

safe_transaction_service/safe_messages/utils.py

@@ -1,22 +1,36 @@
 from typing import Any
 
-from eth_account.messages import defunct_hash_message
+from eth_account.messages import encode_defunct
 from eth_typing import ChecksumAddress, Hash32
 from safe_eth.eth import get_auto_ethereum_client
-from safe_eth.eth.eip712 import eip712_encode_hash
+from safe_eth.eth.eip712 import eip712_encode
 from safe_eth.safe import Safe
 
 
-def get_hash_for_message(message: str | dict[str, Any]) -> Hash32:
+def encode_eip191_message(message: str) -> bytes:
+    signable_message = encode_defunct(text=message)
     return (
-        defunct_hash_message(text=message)
+        b"\x19"
+        + signable_message.version
+        + signable_message.header
+        + signable_message.body
+    )
+
+
+def encode_eip712_message(message: dict[str, Any]) -> bytes:
+    return b"".join(eip712_encode(message))
+
+
+def get_message_encoded(message: str | dict[str, Any]) -> bytes:
+    return (
+        encode_eip191_message(message)
         if isinstance(message, str)
-        else eip712_encode_hash(message)
+        else encode_eip712_message(message)
     )
 
 
 def get_safe_message_hash_for_message(
-    safe_address: ChecksumAddress, message_hash: Hash32
+    safe_address: ChecksumAddress, message: bytes
 ) -> Hash32:
     safe = Safe(safe_address, get_auto_ethereum_client())
-    return safe.get_message_hash(message_hash)
+    return safe.get_message_hash(message)