Fix slow query when getting threshold for multisig-transactions (#1876)

* Fix slow query when getting threshold for multisig-transactions - Getting the threshold for executed transactions was slow, as sorting was done using multiple tables. - Sorting was optimized to use the `internal_tx_id`. As traces are inserted in order and atomically, that should do it, otherwise is not a critical error. - A index was created to optimize `MultisigTransaction` retrieval. Another solution would be to use a Window function, but performance now is quite good. Query went from ```sql Limit (cost=2161.92..23561.22 rows=21 width=1993) (actual time=24695.459..29421.259 rows=21 loops=1) -> Result (cost=2161.92..83326974.35 rows=81770 width=1993) (actual time=24695.458..29421.247 rows=21 loops=1) -> Incremental Sort (cost=2161.92..8347228.91 rows=81770 width=1952) (actual time=177.971..177.991 rows=21 loops=1) Sort Key: history_multisigtransaction.nonce DESC, history_multisigtransaction.created DESC Presorted Key: history_multisigtransaction.nonce Full-sort Groups: 1 Sort Method: quicksort Average Memory: 98kB Peak Memory: 98kB -> Nested Loop Left Join (cost=1.42..8342967.85 rows=81770 width=1952) (actual time=13.972..177.805 rows=22 loops=1) -> Nested Loop Left Join (cost=0.99..7839429.23 rows=81770 width=1862) (actual time=10.742..117.670 rows=22 loops=1) -> Index Scan Backward using history_multisigtransaction_nonce_a98eecaa on history_multisigtransaction (cost=0.43..7227223.48 rows=81770 width=380) (actual time=5.918..11.412 rows=22 loops=1) Filter: (safe = '\xXXXXXXXXXXXXXXXXXXXXXX'::bytea) Rows Removed by Filter: 3 -> Index Scan using history_ethereumtx_pkey on history_ethereumtx (cost=0.56..7.49 rows=1 width=1482) (actual time=4.827..4.827 rows=1 loops=22) Index Cond: (tx_hash = history_multisigtransaction.ethereum_tx_id) -> Index Scan using history_ethereumblock_pkey on history_ethereumblock (cost=0.43..6.16 rows=1 width=90) (actual time=2.730..2.730 rows=1 loops=22) Index Cond: (number = history_ethereumtx.block_id) SubPlan 1 -> Limit (cost=0.42..8.44 rows=1 width=5) (never executed) -> Index Scan using history_safelaststatus_pkey on history_safelaststatus u0 (cost=0.42..8.44 rows=1 width=5) (never executed) Index Cond: (address = history_multisigtransaction.safe) SubPlan 2 -> Limit (cost=606.17..908.51 rows=1 width=46) (actual time=1392.528..1392.528 rows=1 loops=21) -> Incremental Sort (cost=606.17..1210.85 rows=2 width=46) (actual time=1392.526..1392.526 rows=1 loops=21) Sort Key: u0_1.nonce, u1.block_number, u2.transaction_index, u0_1.internal_tx_id Presorted Key: u0_1.nonce Full-sort Groups: 21 Sort Method: quicksort Average Memory: 25kB Peak Memory: 25kB -> Nested Loop (cost=1.55..1210.76 rows=1 width=46) (actual time=1392.407..1392.497 rows=1 loops=21) -> Nested Loop (cost=0.99..1202.18 rows=1 width=75) (actual time=1392.254..1392.343 rows=1 loops=21) -> Index Scan Backward using history_saf_address_aa71bd_idx on history_safestatus u0_1 (cost=0.56..390.22 rows=96 width=38) (actual time=0.378..182.739 rows=83301 loops=21) Index Cond: (address = history_multisigtransaction.safe) -> Index Scan using history_internaltx_pkey on history_internaltx u1 (cost=0.44..8.46 rows=1 width=45) (actual time=0.014..0.014 rows=0 loops=1749321) Index Cond: (id = u0_1.internal_tx_id) Filter: (ethereum_tx_id = history_multisigtransaction.ethereum_tx_id) Rows Removed by Filter: 1 -> Index Scan using history_ethereumtx_pkey on history_ethereumtx u2 (cost=0.56..8.57 rows=1 width=37) (actual time=0.150..0.150 rows=1 loops=21) Index Cond: (tx_hash = history_multisigtransaction.ethereum_tx_id) Planning Time: 84.834 ms Execution Time: 29421.447 ms (37 rows) ``` to ```sql Limit (cost=2161.92..4662.61 rows=21 width=1993) (actual time=0.377..0.485 rows=21 loops=1) -> Result (cost=2161.92..9739363.16 rows=81770 width=1993) (actual time=0.376..0.481 rows=21 loops=1) -> Incremental Sort (cost=2161.92..8347228.91 rows=81770 width=1952) (actual time=0.348..0.350 rows=21 loops=1) Sort Key: history_multisigtransaction.nonce DESC, history_multisigtransaction.created DESC Presorted Key: history_multisigtransaction.nonce Full-sort Groups: 1 Sort Method: quicksort Average Memory: 98kB Peak Memory: 98kB -> Nested Loop Left Join (cost=1.42..8342967.85 rows=81770 width=1952) (actual time=0.041..0.287 rows=22 loops=1) -> Nested Loop Left Join (cost=0.99..7839429.23 rows=81770 width=1862) (actual time=0.032..0.213 rows=22 loops=1) -> Index Scan Backward using history_multisigtransaction_nonce_a98eecaa on history_multisigtransaction (cost=0.43..7227223.48 rows=81770 width=380) (actual time=0.017..0.049 rows=22 loops=1) Filter: (safe = '\xXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXx'::bytea) Rows Removed by Filter: 3 -> Index Scan using history_ethereumtx_pkey on history_ethereumtx (cost=0.56..7.49 rows=1 width=1482) (actual time=0.007..0.007 rows=1 loops=22) Index Cond: (tx_hash = history_multisigtransaction.ethereum_tx_id) -> Index Scan using history_ethereumblock_pkey on history_ethereumblock (cost=0.43..6.16 rows=1 width=90) (actual time=0.003..0.003 rows=1 loops=22) Index Cond: (number = history_ethereumtx.block_id) SubPlan 1 -> Limit (cost=0.42..8.44 rows=1 width=5) (never executed) -> Index Scan using history_safelaststatus_pkey on history_safelaststatus u0 (cost=0.42..8.44 rows=1 width=5) (never executed) Index Cond: (address = history_multisigtransaction.safe) SubPlan 2 -> Limit (cost=0.56..8.57 rows=1 width=13) (actual time=0.005..0.005 rows=1 loops=21) -> Index Scan Backward using history_saf_address_1c362b_idx on history_safestatus u0_1 (cost=0.56..8.57 rows=1 width=13) (actual time=0.005..0.005 rows=1 loops=21) Index Cond: ((address = history_multisigtransaction.safe) AND (nonce = history_multisigtransaction.nonce)) Planning Time: 0.577 ms Execution Time: 0.580 ms (25 rows) ``` * Fix tests --------- Co-authored-by: Uxio Fuentefria <[email protected]>
safe-global · Feb 19, 2024 · 3021db5 · 3021db5
1 parent d176dda
commit 3021db5
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 11 deletions.
diff --git a/...vice/history/migrations/0078_remove_safestatus_history_saf_address_1c362b_idx_and_more.py b/...vice/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",
+            ),
+        ),
+    ]
diff --git a/safe_transaction_service/history/models.py b/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"

diff --git a/safe_transaction_service/history/tests/test_models.py b/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(