Fix wrong domain separator for contract versions below 1.3.0

safe-global · Jan 2, 2023 · 50947cb · 50947cb
1 parent 2bd9573
commit 50947cb
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 12 deletions.
diff --git a/gnosis/safe/safe.py b/gnosis/safe/safe.py
@@ -88,9 +88,12 @@ class Safe:
         0x4A204F620C8C5CCDCA3FD54D003BADD85BA500436A431F0CBDA4F558C93C34C8
     )
     # keccak256("EIP712Domain(uint256 chainId,address verifyingContract)");
-    DOMAIN_TYPEHASH = bytes.fromhex(
+    DOMAIN_TYPEHASH_V1_3_0 = bytes.fromhex(
         "47e79534a245952e8b16893a336b85a3d9ea9fa8c573f3d803afb92a79469218"
     )
+    DOMAIN_TYPEHASH_V1_1_1 = bytes.fromhex(
+        "035aff83d86937d35b32e04f0ddc6ff469290eef2f1b692d8a815c89404d4749"
+    )
     # keccak256("SafeMessage(bytes message)");
     SAFE_MESSAGE_TYPEHASH = bytes.fromhex(
         "60b3cbf8b4a223d68d641b3b6ddf9a298e7f33710cf3d3a9d1146b5a6150fbca"
@@ -519,16 +522,28 @@ def contract(self) -> Contract:
 
     @cached_property
     def domain_separator(self):
-        return fast_keccak(
-            encode_abi(
-                ["bytes32", "uint256", "address"],
-                [
-                    self.DOMAIN_TYPEHASH,
-                    self.ethereum_client.get_chain_id(),
-                    self.address,
-                ],
+        version = self.contract.functions.VERSION().call()
+        if version == "1.3.0":
+            return fast_keccak(
+                encode_abi(
+                    ["bytes32", "uint256", "address"],
+                    [
+                        self.DOMAIN_TYPEHASH_V1_3_0,
+                        self.ethereum_client.get_chain_id(),
+                        self.address,
+                    ],
+                )
+            )
+        else:
+            return fast_keccak(
+                encode_abi(
+                    ["bytes32", "address"],
+                    [
+                        self.DOMAIN_TYPEHASH_V1_1_1,
+                        self.address,
+                    ],
+                )
             )
-        )
 
     def check_funds_for_tx_gas(
         self, safe_tx_gas: int, base_gas: int, gas_price: int, gas_token: str

diff --git a/gnosis/safe/tests/test_signatures.py b/gnosis/safe/tests/test_signatures.py
@@ -11,6 +11,9 @@
 
 
 class TestSafeSignature(SafeTestCaseMixin, TestCase):
+    EIP1271_MAGIC_VALUE = "20c13b0b"
+    UPDATED_MAGIC_VALUE = "1626ba7e"
+
     def test_get_signing_address(self):
         account = Account.create()
         # Random hash
@@ -52,7 +55,7 @@ def test_eip1271_signing(self):
         )
         self.assertEqual(
             is_valid_bytes_fn(message.encode(), signature.signature).call(),
-            bytes.fromhex("20c13b0b"),
+            bytes.fromhex(self.EIP1271_MAGIC_VALUE),
         )
 
         # Use new isValidSignature method (receives bytes32 == hash of the message)
@@ -70,5 +73,31 @@ def test_eip1271_signing(self):
         )
         self.assertEqual(
             is_valid_bytes_fn(message_hash, signature.signature).call(),
-            bytes.fromhex("1626ba7e"),
+            bytes.fromhex(self.UPDATED_MAGIC_VALUE),
+        )
+
+    def test_eip1271_signing_v1_1_1_contract(self):
+        owner = Account.create()
+        message = "luar_na_lubre"
+        safe = self.deploy_test_safe_v1_1_1(threshold=1, owners=[owner.address])
+
+        self.assertEqual(
+            safe.contract.functions.domainSeparator().call(), safe.domain_separator
+        )
+
+        contract = safe.contract
+        safe_message_hash = safe.get_message_hash(message)
+        self.assertEqual(
+            contract.functions.getMessageHash(message.encode()).call(),
+            safe_message_hash,
+        )
+
+        # Use isValidSignature method (receives bytes)
+        signature = owner.signHash(safe_message_hash)
+
+        self.assertEqual(
+            contract.functions.isValidSignature(
+                message.encode(), signature.signature
+            ).call(),
+            bytes.fromhex(self.EIP1271_MAGIC_VALUE),
         )