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..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"), - 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 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(