From 02bbcffd4a57ccf742bb98ed8360bca9e25c98d0 Mon Sep 17 00:00:00 2001 From: Uxio Fuentefria Date: Fri, 16 Feb 2024 17:49:50 +0100 Subject: [PATCH 1/2] 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) ``` --- ...history_saf_address_1c362b_idx_and_more.py | 23 +++++++++++++++++++ safe_transaction_service/history/models.py | 11 ++++++--- 2 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 safe_transaction_service/history/migrations/0078_remove_safestatus_history_saf_address_1c362b_idx_and_more.py diff --git a/safe_transaction_service/history/migrations/0078_remove_safestatus_history_saf_address_1c362b_idx_and_more.py b/safe_transaction_service/history/migrations/0078_remove_safestatus_history_saf_address_1c362b_idx_and_more.py new file mode 100644 index 000000000..aa8b1ff87 --- /dev/null +++ b/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", + ), + ), + ] diff --git a/safe_transaction_service/history/models.py b/safe_transaction_service/history/models.py index 2b9a64113..d5e27a8dd 100644 --- a/safe_transaction_service/history/models.py +++ b/safe_transaction_service/history/models.py @@ -1321,9 +1321,9 @@ def with_confirmations_required(self): 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 +1400,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 +1989,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" From 48fa70a836ec76b41384da8664e3a7fce3a0a7ea Mon Sep 17 00:00:00 2001 From: Uxio Fuentefria <6909403+Uxio0@users.noreply.github.com> Date: Mon, 19 Feb 2024 14:57:56 +0100 Subject: [PATCH 2/2] Fix tests --- safe_transaction_service/history/models.py | 16 +++++++++++- .../history/tests/test_models.py | 25 +++++++++++++------ 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/safe_transaction_service/history/models.py b/safe_transaction_service/history/models.py index d5e27a8dd..38e6aa4e2 100644 --- 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"), nonce=OuterRef("nonce"), ) - .order_by("internal_tx_id") + .order_by("-internal_tx_id") .values("threshold") ) diff --git a/safe_transaction_service/history/tests/test_models.py b/safe_transaction_service/history/tests/test_models.py index 97b9883fc..a309cb5ce 100644 --- 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,8 +1512,8 @@ 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() @@ -1521,7 +1521,16 @@ def test_with_confirmations_required(self): 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(