From 828684515ba08ee46ba4b6ea72ca10664525f808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ux=C3=ADo?= Date: Thu, 7 Nov 2024 12:34:16 +0100 Subject: [PATCH] Revert "Denormalize InternalTxDecoded" This reverts commit 6bbe6475b7856fa7f46f07f1a99e43b8b42e3b3a. --- safe_transaction_service/history/admin.py | 9 ++-- .../history/indexers/internal_tx_indexer.py | 1 - .../history/indexers/safe_events_indexer.py | 19 +++---- .../management/commands/decode_txs_again.py | 1 - .../0091_add_safe_internal_tx_decoded.py | 43 ---------------- safe_transaction_service/history/models.py | 13 +++-- .../history/services/index_service.py | 6 +-- .../history/tests/factories.py | 5 +- .../history/tests/test_index_service.py | 4 +- .../history/tests/test_migrations.py | 49 ------------------- .../history/tests/test_models.py | 27 +++++----- .../history/tests/test_tasks.py | 4 +- .../history/tests/test_tx_processor.py | 48 +++++++++--------- 13 files changed, 67 insertions(+), 162 deletions(-) delete mode 100644 safe_transaction_service/history/migrations/0091_add_safe_internal_tx_decoded.py diff --git a/safe_transaction_service/history/admin.py b/safe_transaction_service/history/admin.py index 750b78d16..67dc0a8b5 100644 --- a/safe_transaction_service/history/admin.py +++ b/safe_transaction_service/history/admin.py @@ -227,7 +227,11 @@ def queryset(self, request, queryset): if self.value() == "YES": return queryset.filter( Q( - Exists(SafeContract.objects.filter(address=OuterRef("safe"))) + Exists( + SafeContract.objects.filter( + address=OuterRef("internal_tx___from") + ) + ) ) # Just Safes indexed | Q(function_name="setup") # Safes pending to be indexed ) @@ -237,7 +241,6 @@ def queryset(self, request, queryset): class InternalTxDecodedAdmin(AdvancedAdminSearchMixin, admin.ModelAdmin): actions = ["process_again"] list_display = ( - "safe", "block_number", "processed", "internal_tx_id", @@ -257,7 +260,7 @@ class InternalTxDecodedAdmin(AdvancedAdminSearchMixin, admin.ModelAdmin): search_fields = [ "==function_name", "==internal_tx__to", - "==safe", + "==internal_tx___from", "==internal_tx__ethereum_tx__tx_hash", "==internal_tx__block_number", ] diff --git a/safe_transaction_service/history/indexers/internal_tx_indexer.py b/safe_transaction_service/history/indexers/internal_tx_indexer.py index 06a6202df..6e66cef9f 100644 --- a/safe_transaction_service/history/indexers/internal_tx_indexer.py +++ b/safe_transaction_service/history/indexers/internal_tx_indexer.py @@ -249,7 +249,6 @@ def _get_internal_txs_to_decode( function_name=function_name, arguments=arguments, processed=False, - safe=internal_tx._from, ) except CannotDecode as exc: logger.debug("Cannot decode %s: %s", data.hex(), exc) diff --git a/safe_transaction_service/history/indexers/safe_events_indexer.py b/safe_transaction_service/history/indexers/safe_events_indexer.py index 01d3488bf..15c3df87e 100644 --- a/safe_transaction_service/history/indexers/safe_events_indexer.py +++ b/safe_transaction_service/history/indexers/safe_events_indexer.py @@ -276,14 +276,11 @@ def _is_setup_indexed(self, safe_address: ChecksumAddress): :param safe_address: :return: ``True`` if ``SafeSetup`` event was processed, ``False`` otherwise """ - return ( - InternalTxDecoded.objects.for_safe(safe_address) - .filter( - function_name="setup", - internal_tx__contract_address=None, - ) - .exists() - ) + return InternalTxDecoded.objects.filter( + function_name="setup", + internal_tx___from=safe_address, + internal_tx__contract_address=None, + ).exists() @transaction.atomic def decode_elements(self, *args) -> List[EventData]: @@ -371,7 +368,6 @@ def _process_decoded_element( internal_tx=internal_tx, function_name="", arguments=args, - safe=safe_address, ) if event_name == "ProxyCreation" or event_name == "SafeSetup": @@ -522,10 +518,10 @@ def _process_safe_creation_events( # Check if were indexed safe_creation_events_addresses = set(safe_addresses_with_creation_events.keys()) indexed_addresses = InternalTxDecoded.objects.filter( - safe__in=safe_creation_events_addresses, + internal_tx___from__in=safe_creation_events_addresses, function_name="setup", internal_tx__contract_address=None, - ).values_list("safe", flat=True) + ).values_list("internal_tx___from", flat=True) # Ignoring the already indexed contracts addresses_to_index = safe_creation_events_addresses - set(indexed_addresses) @@ -586,7 +582,6 @@ def _process_safe_creation_events( internal_tx=internal_tx, function_name="setup", arguments=setup_args, - safe=safe_address, ) internal_txs.append(internal_tx) internal_decoded_txs.append(internal_tx_decoded) diff --git a/safe_transaction_service/history/management/commands/decode_txs_again.py b/safe_transaction_service/history/management/commands/decode_txs_again.py index 988ec8a78..740bfc628 100644 --- a/safe_transaction_service/history/management/commands/decode_txs_again.py +++ b/safe_transaction_service/history/management/commands/decode_txs_again.py @@ -27,7 +27,6 @@ def handle(self, *args, **options): internal_tx=internal_tx, function_name=function_name, arguments=arguments, - safe=internal_tx._from, ) found += 1 self.stdout.write( diff --git a/safe_transaction_service/history/migrations/0091_add_safe_internal_tx_decoded.py b/safe_transaction_service/history/migrations/0091_add_safe_internal_tx_decoded.py deleted file mode 100644 index b67310322..000000000 --- a/safe_transaction_service/history/migrations/0091_add_safe_internal_tx_decoded.py +++ /dev/null @@ -1,43 +0,0 @@ -# Generated by Django 5.0.9 on 2024-11-04 13:16 - -from django.db import migrations, models - -import safe_eth.eth.django.models -from safe_eth.eth.constants import NULL_ADDRESS - - -class Migration(migrations.Migration): - - dependencies = [ - ("history", "0090_multisigtransaction_proposed_by_delegate"), - ] - - operations = [ - migrations.RemoveIndex( - model_name="internaltxdecoded", - name="history_decoded_processed_idx", - ), - migrations.AddField( - model_name="internaltxdecoded", - name="safe", - field=safe_eth.eth.django.models.EthereumAddressBinaryField( - db_index=True, default=NULL_ADDRESS - ), - preserve_default=False, - ), - migrations.RunSQL( - """ - UPDATE "history_internaltxdecoded" decoded SET safe = internal._from - FROM "history_internaltx" internal - WHERE internal.id = decoded.internal_tx_id - """ - ), - migrations.AddIndex( - model_name="internaltxdecoded", - index=models.Index( - condition=models.Q(("processed", False)), - fields=["safe"], - name="history_decoded_processed_idx", - ), - ), - ] diff --git a/safe_transaction_service/history/models.py b/safe_transaction_service/history/models.py index aee87224c..fb07f345f 100644 --- a/safe_transaction_service/history/models.py +++ b/safe_transaction_service/history/models.py @@ -1168,7 +1168,7 @@ def for_safe(self, safe_address: ChecksumAddress): :param safe_address: :return: Queryset of all InternalTxDecoded for one Safe with `safe_address` """ - return self.filter(safe=safe_address) + return self.filter(internal_tx___from=safe_address) def processed(self): return self.filter(processed=True) @@ -1204,7 +1204,7 @@ def pending_for_safe(self, safe_address: ChecksumAddress): """ return ( self.pending_for_safes() - .for_safe(safe_address) + .filter(internal_tx___from=safe_address) .select_related("internal_tx", "internal_tx__ethereum_tx") ) @@ -1212,7 +1212,11 @@ def safes_pending_to_be_processed(self) -> QuerySet[ChecksumAddress]: """ :return: List of Safe addresses that have transactions pending to be processed """ - return self.not_processed().values_list("safe", flat=True).distinct("safe") + return ( + self.not_processed() + .values_list("internal_tx___from", flat=True) + .distinct("internal_tx___from") + ) class InternalTxDecoded(models.Model): @@ -1226,13 +1230,12 @@ class InternalTxDecoded(models.Model): function_name = models.CharField(max_length=256, db_index=True) arguments = JSONField() processed = models.BooleanField(default=False) - safe = EthereumAddressBinaryField(db_index=True) class Meta: indexes = [ models.Index( name="history_decoded_processed_idx", - fields=["safe"], + fields=["processed"], condition=Q(processed=False), ) ] diff --git a/safe_transaction_service/history/services/index_service.py b/safe_transaction_service/history/services/index_service.py index 72988b0ec..1d250efb7 100644 --- a/safe_transaction_service/history/services/index_service.py +++ b/safe_transaction_service/history/services/index_service.py @@ -385,7 +385,7 @@ def _reprocess(self, addresses: List[str]): logger.info("Mark all internal txs decoded as not processed") queryset = InternalTxDecoded.objects.all() if addresses: - queryset = queryset.filter(safe__in=addresses) + queryset = queryset.filter(internal_tx___from__in=addresses) queryset.update(processed=False) @transaction.atomic @@ -414,8 +414,8 @@ def fix_out_of_order( "[%s] Marking InternalTxDecoded newer than timestamp as not processed", address, ) - InternalTxDecoded.objects.for_safe(address).filter( - internal_tx__timestamp__gte=timestamp + InternalTxDecoded.objects.filter( + internal_tx___from=address, internal_tx__timestamp__gte=timestamp ).update(processed=False) logger.info("[%s] Removing SafeStatus newer than timestamp", address) SafeStatus.objects.filter( diff --git a/safe_transaction_service/history/tests/factories.py b/safe_transaction_service/history/tests/factories.py index 878a51f7a..e0b179738 100644 --- a/safe_transaction_service/history/tests/factories.py +++ b/safe_transaction_service/history/tests/factories.py @@ -190,12 +190,9 @@ class Params: "operation": 0, } - internal_tx = factory.SubFactory( - InternalTxFactory, _from=factory.SelfAttribute("..safe") - ) + internal_tx = factory.SubFactory(InternalTxFactory) function_name = factory.fuzzy.FuzzyText(prefix="safe-", suffix="fn") processed = False - safe = factory.LazyFunction(lambda: Account.create().address) @factory.lazy_attribute def arguments(self) -> Dict[str, Any]: diff --git a/safe_transaction_service/history/tests/test_index_service.py b/safe_transaction_service/history/tests/test_index_service.py index 7a1aa19dd..8f023727d 100644 --- a/safe_transaction_service/history/tests/test_index_service.py +++ b/safe_transaction_service/history/tests/test_index_service.py @@ -142,7 +142,7 @@ def test_process_decoded_txs(self): setup_internal_tx = InternalTxDecodedFactory( function_name="setup", - safe=safe_address, + internal_tx___from=safe_address, ) self.assertEqual(self.index_service.process_decoded_txs(safe_address), 1) fix_out_of_order_mock.assert_not_called() @@ -152,7 +152,7 @@ def test_process_decoded_txs(self): exec_transactions = [ InternalTxDecodedFactory( function_name="execTransaction", - safe=safe_address, + internal_tx___from=safe_address, ) for _ in range(3) ] diff --git a/safe_transaction_service/history/tests/test_migrations.py b/safe_transaction_service/history/tests/test_migrations.py index 0437fef41..894d5a7e6 100644 --- a/safe_transaction_service/history/tests/test_migrations.py +++ b/safe_transaction_service/history/tests/test_migrations.py @@ -5,7 +5,6 @@ from django_test_migrations.migrator import Migrator from eth_account import Account -from safe_eth.eth.constants import NULL_ADDRESS from safe_eth.eth.utils import fast_keccak, fast_keccak_text @@ -364,51 +363,3 @@ def test_migration_0082_safecontract_created(self): self.assertEqual( safe_contract.created, safe_contract.ethereum_tx.block.timestamp ) - - def test_migration_0091_add_safe_internal_tx_decoded(self): - # Add `safe` field to InternalTxDecoded - old_state = self.migrator.apply_initial_migration( - ("history", "0090_multisigtransaction_proposed_by_delegate"), - ) - - EthereumBlock = old_state.apps.get_model("history", "EthereumBlock") - EthereumTx = old_state.apps.get_model("history", "EthereumTx") - InternalTx = old_state.apps.get_model("history", "InternalTx") - InternalTxDecoded = old_state.apps.get_model("history", "InternalTxDecoded") - ethereum_tx = self.build_ethereum_tx(EthereumBlock, EthereumTx) - random_safe_address = Account.create().address - internal_tx = InternalTx.objects.create( - ethereum_tx=ethereum_tx, - timestamp=timezone.now(), - block_number=ethereum_tx.block.number, - _from=random_safe_address, - gas=5, - data=b"", - to=NULL_ADDRESS, - value=0, - gas_used=10, - contract_address=None, - code=None, - output=None, - refund_address=None, - tx_type=0, - call_type=0, - trace_address="0", - error=None, - ) - InternalTxDecoded.objects.create( - internal_tx=internal_tx, - function_name="anything", - arguments={}, - processed=True, - ) - - new_state = self.migrator.apply_tested_migration( - ("history", "0091_add_safe_internal_tx_decoded"), - ) - - InternalTxDecodedNew = new_state.apps.get_model("history", "InternalTxDecoded") - internal_tx_decoded_new = InternalTxDecodedNew.objects.get() - self.assertEqual( - internal_tx_decoded_new.safe, internal_tx._from, random_safe_address - ) diff --git a/safe_transaction_service/history/tests/test_models.py b/safe_transaction_service/history/tests/test_models.py index d2bf400d6..87b58974a 100644 --- a/safe_transaction_service/history/tests/test_models.py +++ b/safe_transaction_service/history/tests/test_models.py @@ -665,10 +665,7 @@ def test_internal_txs_can_be_decoded(self): self.assertEqual(InternalTx.objects.can_be_decoded().count(), 1) InternalTxDecoded.objects.create( - function_name="alo", - arguments={}, - internal_tx=internal_tx, - safe=internal_tx._from, + function_name="alo", arguments={}, internal_tx=internal_tx ) self.assertEqual(InternalTx.objects.can_be_decoded().count(), 0) @@ -746,14 +743,18 @@ def test_safes_pending_to_be_processed(self): ) safe_address_1 = SafeContractFactory().address - internal_tx_decoded_1 = InternalTxDecodedFactory(safe=safe_address_1) - InternalTxDecodedFactory(safe=safe_address_1) + internal_tx_decoded_1 = InternalTxDecodedFactory( + internal_tx___from=safe_address_1 + ) + InternalTxDecodedFactory(internal_tx___from=safe_address_1) results = InternalTxDecoded.objects.safes_pending_to_be_processed() self.assertIsInstance(results, QuerySet) self.assertCountEqual(results, [safe_address_1]) safe_address_2 = SafeContractFactory().address - internal_tx_decoded_2 = InternalTxDecodedFactory(safe=safe_address_2) + internal_tx_decoded_2 = InternalTxDecodedFactory( + internal_tx___from=safe_address_2 + ) self.assertCountEqual( InternalTxDecoded.objects.safes_pending_to_be_processed(), [safe_address_1, safe_address_2], @@ -771,7 +772,7 @@ def test_out_of_order_for_safe(self): self.assertFalse(InternalTxDecoded.objects.out_of_order_for_safe(random_safe)) i = InternalTxDecodedFactory( - safe=random_safe, + internal_tx___from=random_safe, internal_tx__block_number=10, processed=False, ) @@ -781,14 +782,14 @@ def test_out_of_order_for_safe(self): self.assertFalse(InternalTxDecoded.objects.out_of_order_for_safe(random_safe)) InternalTxDecodedFactory( - safe=random_safe, + internal_tx___from=random_safe, internal_tx__block_number=11, processed=False, ) self.assertFalse(InternalTxDecoded.objects.out_of_order_for_safe(random_safe)) InternalTxDecodedFactory( - safe=random_safe, internal_tx__block_number=9, processed=False + internal_tx___from=random_safe, internal_tx__block_number=9, processed=False ) self.assertTrue(InternalTxDecoded.objects.out_of_order_for_safe(random_safe)) i.processed = False @@ -797,17 +798,17 @@ def test_out_of_order_for_safe(self): self.assertFalse(InternalTxDecoded.objects.out_of_order_for_safe(random_safe)) InternalTxDecodedFactory( - safe=random_safe, internal_tx__block_number=8, processed=True + internal_tx___from=random_safe, internal_tx__block_number=8, processed=True ) self.assertFalse(InternalTxDecoded.objects.out_of_order_for_safe(random_safe)) InternalTxDecodedFactory( - safe=random_safe, internal_tx__block_number=9, processed=True + internal_tx___from=random_safe, internal_tx__block_number=9, processed=True ) self.assertFalse(InternalTxDecoded.objects.out_of_order_for_safe(random_safe)) InternalTxDecodedFactory( - safe=random_safe, internal_tx__block_number=10, processed=True + internal_tx___from=random_safe, internal_tx__block_number=10, processed=True ) self.assertTrue(InternalTxDecoded.objects.out_of_order_for_safe(random_safe)) diff --git a/safe_transaction_service/history/tests/test_tasks.py b/safe_transaction_service/history/tests/test_tasks.py index 328d7b7cf..769eb1f65 100644 --- a/safe_transaction_service/history/tests/test_tasks.py +++ b/safe_transaction_service/history/tests/test_tasks.py @@ -188,7 +188,7 @@ def test_process_decoded_internal_txs_task(self): threshold=threshold, fallback_handler=fallback_handler, internal_tx__to=master_copy, - safe=safe_address, + internal_tx___from=safe_address, ) process_decoded_internal_txs_task.delay() self.assertTrue(SafeContract.objects.get(address=safe_address)) @@ -211,7 +211,7 @@ def test_process_decoded_internal_txs_for_banned_safe(self): threshold=threshold, fallback_handler=fallback_handler, internal_tx__to=master_copy, - safe=safe_address, + internal_tx___from=safe_address, ) SafeContractFactory(address=safe_address, banned=True) self.assertTrue(SafeContract.objects.get(address=safe_address).banned) diff --git a/safe_transaction_service/history/tests/test_tx_processor.py b/safe_transaction_service/history/tests/test_tx_processor.py index 225d6953e..7255de065 100644 --- a/safe_transaction_service/history/tests/test_tx_processor.py +++ b/safe_transaction_service/history/tests/test_tx_processor.py @@ -68,7 +68,7 @@ def test_tx_processor_with_factory(self): threshold=threshold, fallback_handler=fallback_handler, internal_tx__to=master_copy, - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ) ) @@ -88,14 +88,14 @@ def test_tx_processor_with_factory(self): [ InternalTxDecodedFactory( function_name="execTransaction", - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ), InternalTxDecodedFactory( function_name="addOwnerWithThreshold", owner=new_owner, threshold=threshold, - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ), ] @@ -125,14 +125,14 @@ def test_tx_processor_with_factory(self): [ InternalTxDecodedFactory( function_name="execTransaction", - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ), InternalTxDecodedFactory( function_name="swapOwner", old_owner=owner, owner=another_owner, - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ), ] @@ -185,14 +185,14 @@ def test_tx_processor_with_factory(self): [ InternalTxDecodedFactory( function_name="execTransaction", - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ), InternalTxDecodedFactory( function_name="removeOwner", old_owner=another_owner, threshold=threshold, - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ), ] @@ -228,13 +228,13 @@ def test_tx_processor_with_factory(self): [ InternalTxDecodedFactory( function_name="execTransaction", - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ), InternalTxDecodedFactory( function_name="setFallbackHandler", fallback_handler=fallback_handler, - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ), ] @@ -252,13 +252,13 @@ def test_tx_processor_with_factory(self): [ InternalTxDecodedFactory( function_name="execTransaction", - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ), InternalTxDecodedFactory( function_name="changeMasterCopy", master_copy=master_copy, - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ), ] @@ -276,13 +276,13 @@ def test_tx_processor_with_factory(self): [ InternalTxDecodedFactory( function_name="execTransaction", - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ), InternalTxDecodedFactory( function_name="enableModule", module=module, - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ), ] @@ -298,13 +298,13 @@ def test_tx_processor_with_factory(self): [ InternalTxDecodedFactory( function_name="execTransaction", - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ), InternalTxDecodedFactory( function_name="disableModule", module=module, - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ), ] @@ -328,7 +328,7 @@ def test_tx_processor_with_factory(self): [ InternalTxDecodedFactory( function_name="execTransactionFromModule", - safe=safe_address, + internal_tx___from=safe_address, internal_tx__trace_address="0,0", internal_tx__value=0, ) @@ -355,7 +355,7 @@ def test_tx_processor_with_factory(self): approve_hash_decoded_tx = InternalTxDecodedFactory( function_name="approveHash", hash_to_approve=hash_to_approve, - safe=safe_address, + internal_tx___from=safe_address, internal_tx__trace_address="0,1,0", internal_tx__value=0, ) @@ -374,7 +374,7 @@ def test_tx_processor_with_factory(self): [ InternalTxDecodedFactory( function_name="execTransaction", - safe=safe_address, + internal_tx___from=safe_address, internal_tx__value=0, ), approve_hash_decoded_tx, @@ -489,18 +489,18 @@ def test_tx_processor_change_master_copy(self): threshold=threshold, fallback_handler=fallback_handler, internal_tx__to=safe_1_1_0_master_copy.address, - safe=safe_address, + internal_tx___from=safe_address, ) ) tx_processor.process_decoded_transactions( [ InternalTxDecodedFactory( - function_name="execTransaction", safe=safe_address + function_name="execTransaction", internal_tx___from=safe_address ), InternalTxDecodedFactory( function_name="changeMasterCopy", master_copy=safe_1_2_0_master_copy.address, - safe=safe_address, + internal_tx___from=safe_address, ), ] ) @@ -518,12 +518,12 @@ def test_tx_processor_change_master_copy(self): tx_processor.process_decoded_transactions( [ InternalTxDecodedFactory( - function_name="execTransaction", safe=safe_address + function_name="execTransaction", internal_tx___from=safe_address ), InternalTxDecodedFactory( function_name="changeMasterCopy", master_copy=safe_1_3_0_master_copy.address, - safe=safe_address, + internal_tx___from=safe_address, ), ] ) @@ -541,7 +541,7 @@ def test_process_module_tx(self): safe_last_status = SafeLastStatusFactory() module_internal_tx_decoded = InternalTxDecodedFactory( function_name="execTransactionFromModule", - safe=safe_last_status.address, + internal_tx___from=safe_last_status.address, internal_tx__to="0x34CfAC646f301356fAa8B21e94227e3583Fe3F5F", internal_tx__trace_address="0,0,0,4", internal_tx__ethereum_tx__tx_hash="0x59f20a56a94ad4ee934468eb26b9148151289c97fefece779e05d98befd156f0",