Remove unnecessary Safe call on serializer

- Ensure that 1271 works from 1_1_1.

Author: moisses89

safe_transaction_service/account_abstraction/tests/test_helpers.py

@@ -1,16 +1,14 @@
 from django.test import TestCase
 
 from hexbytes import HexBytes
-from safe_eth.eth.tests.mocks.mock_bundler import (
-    user_operation_mock,
-)
+from safe_eth.eth.tests.mocks.mock_bundler import user_operation_mock
 from safe_eth.safe.tests.safe_test_case import SafeTestCaseMixin
 
 from ..helpers import DecodedInitCode, decode_init_code
 
 
 class TestAccountAbstractionHelpers(SafeTestCaseMixin, TestCase):
-    def test_decode_init_code_v141(self):
+    def test_decode_init_code(self):
         with self.assertRaises(ValueError):
             decode_init_code(b"", self.ethereum_client)
 

safe_transaction_service/history/serializers.py

@@ -34,9 +34,6 @@
     TxDecoderException,
     get_db_tx_decoder,
 )
-from safe_transaction_service.safe_messages.utils import (
-    select_safe_encoded_message_hash_by_safe_version,
-)
 from safe_transaction_service.tokens.serializers import TokenInfoResponseSerializer
 from safe_transaction_service.utils.serializers import (
     EpochDateTimeField,
@@ -122,11 +119,8 @@ def validate_signature(self, signature: bytes):
         )
 
         safe_owners = get_safe_owners(safe_address)
-        safe_hash_preimage = select_safe_encoded_message_hash_by_safe_version(
-            safe.get_version(), safe_tx.safe_tx_hash, safe_tx.safe_tx_hash_preimage
-        )
         parsed_signatures = SafeSignature.parse_signature(
-            signature, safe_tx_hash, safe_hash_preimage=safe_hash_preimage
+            signature, safe_tx_hash, safe_hash_preimage=safe_tx.safe_tx_hash_preimage
         )
         signature_owners = []
         ethereum_client = get_auto_ethereum_client()
@@ -272,13 +266,11 @@ def validate(self, attrs):
         signature_owners = []
         # TODO Make signature mandatory
         signature = attrs.get("signature", b"")
-        # For v1.5.0+, the isValidSignature(bytes32,bytes) expects the original message_hash (bytes32),
-        safe_signature_hash = select_safe_encoded_message_hash_by_safe_version(
-            safe.get_version(), safe_tx.safe_tx_hash, safe_tx.safe_tx_hash_preimage
-        )
+
         parsed_signatures = SafeSignature.parse_signature(
-            signature, safe_tx_hash, safe_hash_preimage=safe_signature_hash
+            signature, safe_tx_hash, safe_hash_preimage=safe_tx.safe_tx_hash_preimage
         )
+
         attrs["parsed_signatures"] = parsed_signatures
         # If there's at least one signature, transaction is trusted (until signatures are mandatory)
         attrs["trusted"] = bool(parsed_signatures)

safe_transaction_service/history/tests/test_views.py

@@ -14,6 +14,7 @@
 from eth_account import Account
 from factory.fuzzy import FuzzyText
 from hexbytes import HexBytes
+from packaging.version import Version
 from requests import ReadTimeout
 from rest_framework import status
 from rest_framework.exceptions import ErrorDetail
@@ -31,9 +32,6 @@
 from safe_transaction_service.contracts.models import ContractQuerySet
 from safe_transaction_service.contracts.tests.factories import ContractFactory
 from safe_transaction_service.contracts.tx_decoder import DbTxDecoder
-from safe_transaction_service.safe_messages.utils import (
-    select_safe_encoded_message_hash_by_safe_version,
-)
 from safe_transaction_service.tokens.models import Token
 from safe_transaction_service.tokens.tests.factories import TokenFactory
 from safe_transaction_service.utils.utils import datetime_to_str
@@ -1773,12 +1771,14 @@ def test_post_multisig_transaction_with_1271_signature(self):
         )
         safe_tx_hash = safe_tx.safe_tx_hash
         safe_tx_hash_preimage = safe_tx.safe_tx_hash_preimage
-
-        selected_hash = select_safe_encoded_message_hash_by_safe_version(
-            safe.get_version(), safe_tx_hash, safe_tx_hash_preimage
+        # >= v1.3.0: supports isValidSignature(bytes32, bytes) and isValidSignature(bytes, bytes)
+        # < v1.3.0: only isValidSignature(bytes, bytes) with the preimage
+        # < v1.5.0: test isValidSignature(bytes, bytes) for backward compatibility
+        safe_owner_message_hash = safe_owner.get_message_hash(
+            safe_tx_hash
+            if Version(safe.get_version()) >= Version("1.5.0")
+            else safe_tx_hash_preimage
         )
-
-        safe_owner_message_hash = safe_owner.get_message_hash(selected_hash)
         safe_owner_signature = account.unsafe_sign_hash(safe_owner_message_hash)[
             "signature"
         ]
@@ -5013,7 +5013,7 @@ def test_export_view_standalone_transfer_priority(self):
 
 class TestViewsV141(TestViewsV150):
     """
-    Test views v2 with Safe v1.4.1 contracts.
+    Test views with Safe v1.4.1 contracts.
     """
 
     def deploy_test_safe(self, *args, **kwargs) -> Safe:
@@ -5023,3 +5023,31 @@ def deploy_test_safe(self, *args, **kwargs) -> Safe:
         :return: Deploy last available Safe
         """
         return self.deploy_test_safe_v1_4_1(*args, **kwargs)
+
+
+class TestViewsV130(TestViewsV150):
+    """
+    Test views with Safe v1.3.0 contracts.
+    """
+
+    def deploy_test_safe(self, *args, **kwargs) -> Safe:
+        """
+        :param args:
+        :param kwargs:
+        :return: Deploy last available Safe
+        """
+        return self.deploy_test_safe_v1_3_0(*args, **kwargs)
+
+
+class TestViewsV111(TestViewsV150):
+    """
+    Test views with Safe v1.1.1 contracts.
+    """
+
+    def deploy_test_safe(self, *args, **kwargs) -> Safe:
+        """
+        :param args:
+        :param kwargs:
+        :return: Deploy last available Safe
+        """
+        return self.deploy_test_safe_v1_1_1(*args, **kwargs)

safe_transaction_service/history/tests/test_views_v2.py

@@ -11,6 +11,7 @@
 import eth_abi
 from eth_account import Account
 from hexbytes import HexBytes
+from packaging.version import Version
 from rest_framework import status
 from rest_framework.exceptions import ErrorDetail
 from rest_framework.test import APIRequestFactory, APITestCase, force_authenticate
@@ -26,9 +27,6 @@
 from ...contracts.models import ContractQuerySet
 from ...contracts.tests.factories import ContractFactory
 from ...contracts.tx_decoder import DbTxDecoder
-from ...safe_messages.utils import (
-    select_safe_encoded_message_hash_by_safe_version,
-)
 from ...tokens.models import Token
 from ...tokens.tests.factories import TokenFactory
 from ...utils.utils import datetime_to_str
@@ -1535,11 +1533,14 @@ def test_post_multisig_transaction_with_1271_signature(self):
         safe_tx_hash = safe_tx.safe_tx_hash
         safe_tx_hash_preimage = safe_tx.safe_tx_hash_preimage
 
-        selected_hash = select_safe_encoded_message_hash_by_safe_version(
-            safe.get_version(), safe_tx_hash, safe_tx_hash_preimage
+        # >= v1.3.0: supports isValidSignature(bytes32, bytes) and isValidSignature(bytes, bytes)
+        # < v1.3.0: only isValidSignature(bytes, bytes) with the preimage
+        # < v1.5.0: test isValidSignature(bytes, bytes) for backward compatibility
+        safe_owner_message_hash = safe_owner.get_message_hash(
+            safe_tx_hash
+            if Version(safe.get_version()) >= Version("1.5.0")
+            else safe_tx_hash_preimage
         )
-
-        safe_owner_message_hash = safe_owner.get_message_hash(selected_hash)
         safe_owner_signature = account.unsafe_sign_hash(safe_owner_message_hash)[
             "signature"
         ]
@@ -2822,3 +2823,17 @@ def deploy_test_safe(self, *args, **kwargs) -> Safe:
         :return: Deploy last available Safe
         """
         return self.deploy_test_safe_v1_4_1(*args, **kwargs)
+
+
+class TestViewsV2V130(TestViewsV2V150):
+    """
+    Test views v2 with Safe v1.3.0 contracts.
+    """
+
+    def deploy_test_safe(self, *args, **kwargs) -> Safe:
+        """
+        :param args:
+        :param kwargs:
+        :return: Deploy last available Safe
+        """
+        return self.deploy_test_safe_v1_3_0(*args, **kwargs)

safe_transaction_service/safe_messages/serializers.py

@@ -12,7 +12,6 @@
 from safe_eth.eth import get_auto_ethereum_client
 from safe_eth.eth.eip712 import eip712_encode
 from safe_eth.eth.utils import fast_keccak
-from safe_eth.safe.safe import Safe
 from safe_eth.safe.safe_signature import SafeSignature, SafeSignatureType
 from safe_eth.util.util import to_0x_hex_str
 
@@ -21,11 +20,7 @@
 )
 
 from .models import SIGNATURE_LENGTH, SafeMessage, SafeMessageConfirmation
-from .utils import (
-    get_message_encoded,
-    get_safe_message_hash_and_preimage_for_message,
-    select_safe_encoded_message_hash_by_safe_version,
-)
+from .utils import get_message_encoded, get_safe_message_hash_and_preimage_for_message
 
 
 # Request serializers
@@ -130,19 +125,10 @@ def validate(self, attrs):
                 f"Message with hash {to_0x_hex_str(safe_message_hash)} for safe {safe_address} already exists in DB"
             )
 
-        # Preimage is encoded for the Safe. But if an EIP-1271 signature is used, owner's Safe will be called
-        # the preimage will be encoded again for the owner Safe. That's what needs to be signed by the user
-        # So original data -> EIP-191 or EIP-712 encoded -> Safe encoded data -> Owner encoded data
-        # For v1.5.0+, the isValidSignature(bytes32,bytes) expects the original message_hash (bytes32),
-        # not the Safe-encoded hash
-        ethereum_client = get_auto_ethereum_client()
-        safe = Safe(safe_address, ethereum_client)
-        safe_message_preimage = select_safe_encoded_message_hash_by_safe_version(
-            safe.get_version(), safe_message_hash, safe_message_preimage
-        )
         safe_signatures = SafeSignature.parse_signature(
             signature, safe_message_hash, safe_hash_preimage=safe_message_preimage
         )
+
         owner, signature_type = self.get_valid_owner_from_signatures(
             safe_signatures, safe_address, None
         )
@@ -185,10 +171,6 @@ def validate(self, attrs):
         )
         assert to_0x_hex_str(safe_message_hash) == safe_message.message_hash
 
-        safe = Safe(safe_address, get_auto_ethereum_client())
-        safe_message_preimage = select_safe_encoded_message_hash_by_safe_version(
-            safe.get_version(), safe_message_hash, safe_message_preimage
-        )
         safe_signatures = SafeSignature.parse_signature(
             signature, safe_message_hash, safe_hash_preimage=safe_message_preimage
         )

safe_transaction_service/safe_messages/tests/test_views.py

@@ -8,6 +8,7 @@
 import eth_abi
 from eth_account import Account
 from hexbytes import HexBytes
+from packaging.version import Version
 from rest_framework import status
 from rest_framework.exceptions import ErrorDetail
 from rest_framework.test import APITestCase
@@ -28,11 +29,7 @@
 )
 from safe_transaction_service.utils.utils import datetime_to_str
 
-from ..utils import (
-    encode_eip191_message,
-    encode_eip712_message,
-    select_safe_encoded_message_hash_by_safe_version,
-)
+from ..utils import encode_eip191_message, encode_eip712_message
 from .mocks import get_eip712_payload_mock
 
 logger = logging.getLogger(__name__)
@@ -299,11 +296,15 @@ def _test_safe_messages_create_using_1271_signature_view(self, safe_deployment_f
             fast_keccak(message_encoded)
         )
 
-        data = select_safe_encoded_message_hash_by_safe_version(
-            safe.get_version(), safe_message_hash, safe_message_preimage
+        # >= v1.3.0: supports isValidSignature(bytes32, bytes) and isValidSignature(bytes, bytes)
+        # < v1.3.0: only isValidSignature(bytes, bytes) with the preimage
+        # < v1.5.0: test isValidSignature(bytes, bytes) for backward compatibility
+        safe_owner_message_hash = safe_owner.get_message_hash(
+            safe_message_hash
+            if Version(safe.get_version()) >= Version("1.5.0")
+            else safe_message_preimage
         )
 
-        safe_owner_message_hash, _ = safe_owner.get_message_hash_and_preimage(data)
         safe_owner_signature = account.unsafe_sign_hash(safe_owner_message_hash)[
             "signature"
         ]
@@ -334,6 +335,11 @@ def _test_safe_messages_create_using_1271_signature_view(self, safe_deployment_f
             SafeMessageConfirmation.objects.get().owner, safe_owner.address
         )
 
+    def test_safe_messages_create_using_1271_signature_v1_1_1_view(self):
+        return self._test_safe_messages_create_using_1271_signature_view(
+            self.deploy_test_safe_v1_1_1
+        )
+
     def test_safe_messages_create_using_1271_signature_v1_3_0_view(self):
         return self._test_safe_messages_create_using_1271_signature_view(
             self.deploy_test_safe_v1_3_0

safe_transaction_service/safe_messages/utils.py

@@ -2,7 +2,6 @@
 
 from eth_account.messages import encode_defunct
 from eth_typing import ChecksumAddress, Hash32
-from packaging.version import Version
 from safe_eth.eth import get_auto_ethereum_client
 from safe_eth.eth.eip712 import eip712_encode
 from safe_eth.safe import Safe
@@ -35,18 +34,3 @@ def get_safe_message_hash_and_preimage_for_message(
 ) -> tuple[Hash32, bytes]:
     safe = Safe(safe_address, get_auto_ethereum_client())
     return safe.get_message_hash_and_preimage(message)
-
-
-def select_safe_encoded_message_hash_by_safe_version(
-    safe_version: str, safe_message_hash: bytes, safe_message_preimage: bytes
-) -> bytes:
-    """
-    Returns the Safe-encoded message hash for v1.5.0+ Safes, otherwise returns the provided preimage.
-
-    :param safe_version: Version of the Safe contract.
-    :param safe_message_preimage: Safe-encoded message hash.
-    :return: Hash to be used for signature validation.
-    """
-    if Version(safe_version) >= Version("1.5.0"):
-        return safe_message_hash
-    return safe_message_preimage