Add exception for trusted contracts for delegate calls when creation not allowed (#2444)

* Add exception for trusted_addresses_for_delegate_call when creation check is disable

* Fix test

Author: falvaradorodriguez

safe_transaction_service/history/serializers.py

@@ -41,6 +41,7 @@
     get_safe_owners,
 )
 
+from ..contracts.models import Contract
 from .exceptions import InternalValidationError, NodeConnectionException
 from .helpers import (
     DelegateSignatureHelper,
@@ -193,26 +194,27 @@ def validate_origin(self, origin):
 
         return origin
 
-    def validate_operation(self, value):
+    def validate(self, attrs):
+        super().validate(attrs)
+
+        tx_to = attrs["to"]
+        tx_operation = attrs["operation"]
         if (
             settings.DISABLE_CREATION_MULTISIG_TRANSACTIONS_WITH_DELEGATE_CALL_OPERATION
-            and value == SafeOperationEnum.DELEGATE_CALL.value
+            and tx_operation == SafeOperationEnum.DELEGATE_CALL.value
+            and tx_to not in Contract.objects.trusted_addresses_for_delegate_call()
         ):
             raise ValidationError("Operation DELEGATE_CALL is not allowed")
-        return super().validate_operation(value)
-
-    def validate(self, attrs):
-        super().validate(attrs)
 
         ethereum_client = get_auto_ethereum_client()
         safe_address = attrs["safe"]
 
         safe = Safe(safe_address, ethereum_client)
         safe_tx = safe.build_multisig_tx(
-            attrs["to"],
+            tx_to,
             attrs["value"],
             attrs["data"],
-            attrs["operation"],
+            tx_operation,
             attrs["safe_tx_gas"],
             attrs["base_gas"],
             attrs["gas_price"],

safe_transaction_service/history/tests/test_views.py

@@ -1383,10 +1383,10 @@ def test_get_multisig_transactions_not_decoded(
             self.assertEqual(response.status_code, status.HTTP_200_OK)
             self.assertIsNone(response.data["results"][0]["data_decoded"])
 
-            ContractQuerySet.cache_trusted_addresses_for_delegate_call.clear()
             ContractFactory(
                 address=multisig_transaction.to, trusted_for_delegate_call=True
             )
+            ContractQuerySet.cache_trusted_addresses_for_delegate_call.clear()
             # Force don't use cache because we are not cleaning the cache on contracts change
             with mock.patch(
                 "safe_transaction_service.history.views.settings.CACHE_VIEW_DEFAULT_TIMEOUT",
@@ -2332,62 +2332,82 @@ def test_post_multisig_transaction_with_delegate_call(self):
         safe_owner_1 = Account.create()
         safe = self.deploy_test_safe(owners=[safe_owner_1.address])
         safe_address = safe.address
-
-        response = self.client.get(
-            reverse("v1:history:multisig-transactions", args=(safe_address,)),
-            format="json",
-        )
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertEqual(response.data["count"], 0)
-
-        data = {
-            "to": Account.create().address,
-            "value": 0,
-            "data": "0x12121212",
-            "operation": 1,
-            "nonce": 0,
-            "safeTxGas": 0,
-            "baseGas": 0,
-            "gasPrice": 0,
-            "gasToken": "0x0000000000000000000000000000000000000000",
-            "refundReceiver": "0x0000000000000000000000000000000000000000",
-            "sender": safe_owner_1.address,
-        }
-        safe_tx = safe.build_multisig_tx(
-            data["to"],
-            data["value"],
-            data["data"],
-            data["operation"],
-            data["safeTxGas"],
-            data["baseGas"],
-            data["gasPrice"],
-            data["gasToken"],
-            data["refundReceiver"],
-            safe_nonce=data["nonce"],
-        )
-        data["contractTransactionHash"] = to_0x_hex_str(safe_tx.safe_tx_hash)
-
-        with self.settings(
-            DISABLE_CREATION_MULTISIG_TRANSACTIONS_WITH_DELEGATE_CALL_OPERATION=True
-        ):
-            response = self.client.post(
+        try:
+            response = self.client.get(
                 reverse("v1:history:multisig-transactions", args=(safe_address,)),
                 format="json",
-                data=data,
             )
-            self.assertEqual(response.status_code, status.HTTP_422_UNPROCESSABLE_ENTITY)
-
-        with self.settings(
-            DISABLE_CREATION_MULTISIG_TRANSACTIONS_WITH_DELEGATE_CALL_OPERATION=False
-        ):
-            response = self.client.post(
-                reverse("v1:history:multisig-transactions", args=(safe_address,)),
-                format="json",
-                data=data,
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+            self.assertEqual(response.data["count"], 0)
+
+            data = {
+                "to": Account.create().address,
+                "value": 0,
+                "data": "0x12121212",
+                "operation": SafeOperationEnum.DELEGATE_CALL.value,
+                "nonce": 0,
+                "safeTxGas": 0,
+                "baseGas": 0,
+                "gasPrice": 0,
+                "gasToken": "0x0000000000000000000000000000000000000000",
+                "refundReceiver": "0x0000000000000000000000000000000000000000",
+                "sender": safe_owner_1.address,
+            }
+            safe_tx = safe.build_multisig_tx(
+                data["to"],
+                data["value"],
+                data["data"],
+                data["operation"],
+                data["safeTxGas"],
+                data["baseGas"],
+                data["gasPrice"],
+                data["gasToken"],
+                data["refundReceiver"],
+                safe_nonce=data["nonce"],
             )
-            self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-            multisig_transaction_db = MultisigTransaction.objects.first()
-            self.assertEqual(multisig_transaction_db.operation, 1)
+            data["contractTransactionHash"] = to_0x_hex_str(safe_tx.safe_tx_hash)
+
+            ContractQuerySet.cache_trusted_addresses_for_delegate_call.clear()
+            # Disable creation with delegate call and not trusted contract
+            with self.settings(
+                DISABLE_CREATION_MULTISIG_TRANSACTIONS_WITH_DELEGATE_CALL_OPERATION=True
+            ):
+                response = self.client.post(
+                    reverse("v1:history:multisig-transactions", args=(safe_address,)),
+                    format="json",
+                    data=data,
+                )
+                self.assertEqual(
+                    response.status_code, status.HTTP_422_UNPROCESSABLE_ENTITY
+                )
+
+            # Enable creation with delegate call
+            with self.settings(
+                DISABLE_CREATION_MULTISIG_TRANSACTIONS_WITH_DELEGATE_CALL_OPERATION=False
+            ):
+                response = self.client.post(
+                    reverse("v1:history:multisig-transactions", args=(safe_address,)),
+                    format="json",
+                    data=data,
+                )
+                self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+                multisig_transaction_db = MultisigTransaction.objects.first()
+                self.assertEqual(multisig_transaction_db.operation, 1)
+
+            # Disable creation with delegate call and trusted contract
+            ContractFactory(address=data["to"], trusted_for_delegate_call=True)
+            ContractQuerySet.cache_trusted_addresses_for_delegate_call.clear()
+            with self.settings(
+                DISABLE_CREATION_MULTISIG_TRANSACTIONS_WITH_DELEGATE_CALL_OPERATION=True
+            ):
+                response = self.client.post(
+                    reverse("v1:history:multisig-transactions", args=(safe_address,)),
+                    format="json",
+                    data=data,
+                )
+                self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        finally:
+            ContractQuerySet.cache_trusted_addresses_for_delegate_call.clear()
 
     def test_safe_balances_view(self):
         safe_address = Account.create().address

safe_transaction_service/history/tests/test_views_v2.py

@@ -1958,61 +1958,82 @@ def test_post_multisig_transaction_with_delegate_call(self):
         safe = self.deploy_test_safe(owners=[safe_owner_1.address])
         safe_address = safe.address
 
-        response = self.client.get(
-            reverse("v2:history:multisig-transactions", args=(safe_address,)),
-            format="json",
-        )
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertEqual(response.data["count"], 0)
-
-        data = {
-            "to": Account.create().address,
-            "value": 0,
-            "data": "0x12121212",
-            "operation": 1,
-            "nonce": 0,
-            "safeTxGas": 0,
-            "baseGas": 0,
-            "gasPrice": 0,
-            "gasToken": "0x0000000000000000000000000000000000000000",
-            "refundReceiver": "0x0000000000000000000000000000000000000000",
-            "sender": safe_owner_1.address,
-        }
-        safe_tx = safe.build_multisig_tx(
-            data["to"],
-            data["value"],
-            data["data"],
-            data["operation"],
-            data["safeTxGas"],
-            data["baseGas"],
-            data["gasPrice"],
-            data["gasToken"],
-            data["refundReceiver"],
-            safe_nonce=data["nonce"],
-        )
-        data["contractTransactionHash"] = to_0x_hex_str(safe_tx.safe_tx_hash)
-
-        with self.settings(
-            DISABLE_CREATION_MULTISIG_TRANSACTIONS_WITH_DELEGATE_CALL_OPERATION=True
-        ):
-            response = self.client.post(
+        try:
+            response = self.client.get(
                 reverse("v2:history:multisig-transactions", args=(safe_address,)),
                 format="json",
-                data=data,
             )
-            self.assertEqual(response.status_code, status.HTTP_422_UNPROCESSABLE_ENTITY)
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+            self.assertEqual(response.data["count"], 0)
 
-        with self.settings(
-            DISABLE_CREATION_MULTISIG_TRANSACTIONS_WITH_DELEGATE_CALL_OPERATION=False
-        ):
-            response = self.client.post(
-                reverse("v2:history:multisig-transactions", args=(safe_address,)),
-                format="json",
-                data=data,
+            data = {
+                "to": Account.create().address,
+                "value": 0,
+                "data": "0x12121212",
+                "operation": SafeOperationEnum.DELEGATE_CALL.value,
+                "nonce": 0,
+                "safeTxGas": 0,
+                "baseGas": 0,
+                "gasPrice": 0,
+                "gasToken": "0x0000000000000000000000000000000000000000",
+                "refundReceiver": "0x0000000000000000000000000000000000000000",
+                "sender": safe_owner_1.address,
+            }
+            safe_tx = safe.build_multisig_tx(
+                data["to"],
+                data["value"],
+                data["data"],
+                data["operation"],
+                data["safeTxGas"],
+                data["baseGas"],
+                data["gasPrice"],
+                data["gasToken"],
+                data["refundReceiver"],
+                safe_nonce=data["nonce"],
             )
-            self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-            multisig_transaction_db = MultisigTransaction.objects.first()
-            self.assertEqual(multisig_transaction_db.operation, 1)
+            data["contractTransactionHash"] = to_0x_hex_str(safe_tx.safe_tx_hash)
+
+            ContractQuerySet.cache_trusted_addresses_for_delegate_call.clear()
+            # Disable creation with delegate call and not trusted contract
+            with self.settings(
+                DISABLE_CREATION_MULTISIG_TRANSACTIONS_WITH_DELEGATE_CALL_OPERATION=True
+            ):
+                response = self.client.post(
+                    reverse("v2:history:multisig-transactions", args=(safe_address,)),
+                    format="json",
+                    data=data,
+                )
+                self.assertEqual(
+                    response.status_code, status.HTTP_422_UNPROCESSABLE_ENTITY
+                )
+
+            # Enable creation with delegate call
+            with self.settings(
+                DISABLE_CREATION_MULTISIG_TRANSACTIONS_WITH_DELEGATE_CALL_OPERATION=False
+            ):
+                response = self.client.post(
+                    reverse("v2:history:multisig-transactions", args=(safe_address,)),
+                    format="json",
+                    data=data,
+                )
+                self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+                multisig_transaction_db = MultisigTransaction.objects.first()
+                self.assertEqual(multisig_transaction_db.operation, 1)
+
+            # Disable creation with delegate call and trusted contract
+            ContractQuerySet.cache_trusted_addresses_for_delegate_call.clear()
+            ContractFactory(address=data["to"], trusted_for_delegate_call=True)
+            with self.settings(
+                DISABLE_CREATION_MULTISIG_TRANSACTIONS_WITH_DELEGATE_CALL_OPERATION=True
+            ):
+                response = self.client.post(
+                    reverse("v2:history:multisig-transactions", args=(safe_address,)),
+                    format="json",
+                    data=data,
+                )
+                self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        finally:
+            ContractQuerySet.cache_trusted_addresses_for_delegate_call.clear()
 
     def test_delete_multisig_transaction(self):
         owner_account = Account.create()