Fix slow query when getting threshold for multisig-transactions

safe_transaction_service/history/migrations/0078_remove_safestatus_history_saf_address_1c362b_idx_and_more.py

@@ -0,0 +1,23 @@
+# Generated by Django 4.2.7 on 2024-02-16 17:15
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("history", "0077_alter_safecontractdelegate_delegate_and_more"),
+    ]
+
+    operations = [
+        migrations.RemoveIndex(
+            model_name="safestatus",
+            name="history_saf_address_1c362b_idx",
+        ),
+        migrations.AddIndex(
+            model_name="multisigtransaction",
+            index=models.Index(
+                fields=["safe", "-nonce", "-created"],
+                name="history_multisigtx_safe_sorted",
+            ),
+        ),
+    ]

safe_transaction_service/history/models.py

@@ -1318,12 +1318,26 @@ def with_confirmations_required(self):
 
         :return: queryset with `confirmations_required: int` field
         """
+
+        """
+        SafeStatus works the following way:
+            - First entry of any Multisig Transactions is `execTransaction`, that increments the nonce.
+            - Next entries are configuration changes on the Safe.
+        For example, for a Multisig Transaction with nonce 1 changing the threshold the `SafeStatus` table
+        will look like:
+            - setup with nonce 0
+            - execTransaction with nonce already increased to 1 for a previous Multisig Transaction.
+            - execTransaction with nonce already increased to 2, old threshold and internal_tx_id=7 (auto increased id).
+            - changeThreshold with nonce already increased to 2, new threshold and internal_tx_id=8 (any number
+              higher than 7).
+        We need to get the previous entry to get the proper threshold at that point before it's changed.
+        """
         threshold_safe_status_query = (
             SafeStatus.objects.filter(
                 address=OuterRef("safe"),
-                internal_tx__ethereum_tx=OuterRef("ethereum_tx"),
+                nonce=OuterRef("nonce"),
             )
-            .sorted_reverse_by_mined()
+            .order_by("-internal_tx_id")
             .values("threshold")
         )
 
@@ -1400,6 +1414,12 @@ class Meta:
         permissions = [
             ("create_trusted", "Can create trusted transactions"),
         ]
+        indexes = [
+            Index(
+                name="history_multisigtx_safe_sorted",
+                fields=["safe", "-nonce", "-created"],
+            ),
+        ]
 
     def __str__(self):
         return f"{self.safe} - {self.nonce} - {self.safe_tx_hash}"
@@ -1983,7 +2003,6 @@ class SafeStatus(SafeStatusBase):
     class Meta:
         indexes = [
             Index(fields=["address", "-nonce"]),  # Index on address and nonce DESC
-            Index(fields=["address", "-nonce", "-internal_tx"]),  # For Window search
         ]
         unique_together = (("internal_tx", "address"),)
         verbose_name_plural = "Safe statuses"

safe_transaction_service/history/tests/test_models.py

@@ -1482,17 +1482,17 @@ def test_not_indexed_metadata_contract_addresses(self):
         )
 
     def test_with_confirmations_required(self):
-        # This should never be picked
+        # This should never be picked, Safe not matching
         SafeStatusFactory(nonce=0, threshold=4)
 
-        multisig_transaction = MultisigTransactionFactory()
+        multisig_transaction = MultisigTransactionFactory(nonce=0)
         self.assertIsNone(
             MultisigTransaction.objects.with_confirmations_required()
             .first()
             .confirmations_required
         )
 
-        # SafeStatus not matching the EthereumTx
+        # SafeStatus not matching the nonce (looking for threshold in nonce=0)
         safe_status = SafeStatusFactory(
             address=multisig_transaction.safe, nonce=1, threshold=8
         )
@@ -1502,8 +1502,8 @@ def test_with_confirmations_required(self):
             .confirmations_required
         )
 
-        safe_status.internal_tx.ethereum_tx = multisig_transaction.ethereum_tx
-        safe_status.internal_tx.save(update_fields=["ethereum_tx"])
+        safe_status.nonce = 0
+        safe_status.save(update_fields=["nonce"])
 
         self.assertEqual(
             MultisigTransaction.objects.with_confirmations_required()
@@ -1512,16 +1512,25 @@ def test_with_confirmations_required(self):
             8,
         )
 
-        # It will not be picked, as EthereumTx is not matching
-        SafeStatusFactory(nonce=2, threshold=15)
+        # It will not be picked, as nonce is still matching the previous SafeStatus
+        SafeStatusFactory(address=multisig_transaction.safe, nonce=1, threshold=15)
         self.assertEqual(
             MultisigTransaction.objects.with_confirmations_required()
             .first()
             .confirmations_required,
             8,
         )
 
-        # As EthereumTx is empty, the latest safe status will be used if available
+        multisig_transaction.nonce = 1
+        multisig_transaction.save(update_fields=["nonce"])
+        self.assertEqual(
+            MultisigTransaction.objects.with_confirmations_required()
+            .first()
+            .confirmations_required,
+            15,
+        )
+
+        # As EthereumTx is empty, the latest Safe Status will be used if available
         multisig_transaction.ethereum_tx = None
         multisig_transaction.save(update_fields=["ethereum_tx"])
         self.assertIsNone(