Improve safe_contract command

Uxio0 · Uxio0 · commit 3027a43f6374 · 2025-12-17T17:05:51.000+01:00
diff --git a/safe_transaction_service/history/indexers/safe_events_indexer.py b/safe_transaction_service/history/indexers/safe_events_indexer.py
@@ -68,7 +68,11 @@ def __init__(self, *args, **kwargs):
         kwargs.setdefault(
             "eth_zksync_compatible_network", settings.ETH_ZKSYNC_COMPATIBLE_NETWORK
         )
+        kwargs.setdefault("ignored_initiators", settings.ETH_EVENTS_IGNORED_INITIATORS)
+
         self.eth_zksync_compatible_network = kwargs["eth_zksync_compatible_network"]
+        self.ignored_initiators = kwargs["ignored_initiators"]
+        self.conditional_indexing_enabled = bool(self.ignored_initiators)
         # Cache timestamp for block hashes
         self.block_hashes_with_timestamp: dict[bytes, datetime.datetime] = {}
         super().__init__(*args, **kwargs)
@@ -81,11 +85,7 @@ def process_elements(self, log_receipts: Sequence[LogReceipt]) -> list[InternalT
         if not log_receipts:
             return []
 
-        if not settings.ETH_EVENTS_CONDITIONAL_INDEXING:
-            return super().process_elements(log_receipts)
-
-        ignored_initiators = settings.ETH_EVENTS_IGNORED_INITIATORS
-        if not ignored_initiators:
+        if not self.ignored_initiators:
             # No blocklist configured, use standard flow
             return super().process_elements(log_receipts)
 
@@ -135,7 +135,7 @@ def process_elements(self, log_receipts: Sequence[LogReceipt]) -> list[InternalT
 
         # Check existing DB txs
         for tx_hash, db_tx in db_txs.items():
-            if db_tx._from not in ignored_initiators:
+            if db_tx._from not in self.ignored_initiators:
                 allowed_tx_hashes.add(tx_hash)
             else:
                 logger.debug(
@@ -148,7 +148,7 @@ def process_elements(self, log_receipts: Sequence[LogReceipt]) -> list[InternalT
         allowed_fetched_txs: list[TxData] = []
         for tx in fetched_txs:
             tx_from = tx["from"]
-            if tx_from not in ignored_initiators:
+            if tx_from not in self.ignored_initiators:
                 allowed_fetched_txs.append(tx)
                 allowed_tx_hashes.add(HexBytes(tx["hash"]))
             else:
@@ -883,7 +883,7 @@ def _process_decoded_elements(
         # When conditional indexing is enabled, filter non-creation events
         # to only process events for Safes that exist in SafeContract table
         elements_to_process = decoded_elements
-        if settings.ETH_EVENTS_CONDITIONAL_INDEXING:
+        if self.conditional_indexing_enabled:
             # Get all unique Safe addresses from non-creation events
             non_creation_addresses = {
                 element["address"]
diff --git a/safe_transaction_service/history/management/commands/safe_contract.py b/safe_transaction_service/history/management/commands/safe_contract.py
@@ -2,10 +2,12 @@
 
 from eth_typing import ChecksumAddress
 from hexbytes import HexBytes
+from safe_eth.eth import get_auto_ethereum_client
 from web3 import Web3
 
-from ...models import EthereumTx, SafeContract
-from ...services import IndexServiceProvider
+from ...indexers.safe_events_indexer import SafeEventsIndexer
+from ...indexers.tx_processor import SafeTxProcessorProvider
+from ...models import InternalTxDecoded, SafeContract
 
 
 class Command(BaseCommand):
@@ -69,7 +71,7 @@ def _validate_tx_hash(self, tx_hash: str) -> bytes:
             raise CommandError(f"Invalid transaction hash format: {tx_hash}") from e
 
     def _handle_add(self, address: str, tx_hash: str) -> None:
-        """Add a SafeContract entry manually."""
+        """Add a SafeContract entry by processing creation events through the indexer."""
         validated_address = self._validate_address(address)
         validated_tx_hash = self._validate_tx_hash(tx_hash)
 
@@ -82,32 +84,52 @@ def _handle_add(self, address: str, tx_hash: str) -> None:
             )
             return
 
-        # Check if EthereumTx exists, fetch from RPC if not
-        try:
-            ethereum_tx = EthereumTx.objects.get(tx_hash=validated_tx_hash)
-            self.stdout.write(f"Found existing EthereumTx for hash {tx_hash}")
-        except EthereumTx.DoesNotExist:
-            self.stdout.write(f"Fetching transaction {tx_hash} from RPC...")
-            index_service = IndexServiceProvider()
-            try:
-                index_service.txs_create_or_update_from_tx_hashes([validated_tx_hash])
-                ethereum_tx = EthereumTx.objects.get(tx_hash=validated_tx_hash)
-                self.stdout.write(
-                    self.style.SUCCESS("Successfully fetched and stored transaction")
-                )
-            except Exception as e:
-                raise CommandError(
-                    f"Failed to fetch transaction {tx_hash} from RPC: {e}"
-                ) from e
+        # Fetch transaction receipt from RPC
+        self.stdout.write(f"Fetching transaction receipt for {tx_hash} from RPC...")
+        ethereum_client = get_auto_ethereum_client()
+        tx_receipt = ethereum_client.get_transaction_receipt(validated_tx_hash)
+        if not tx_receipt:
+            raise CommandError(f"Transaction {tx_hash} not found")
+
+        # Get logs from the receipt
+        logs = tx_receipt.get("logs", [])
+        if not logs:
+            raise CommandError(f"Transaction {tx_hash} has no logs")
+
+        self.stdout.write(f"Found {len(logs)} logs in transaction receipt")
+
+        # Process logs through SafeEventsIndexer
+        # Pass ignored_initiators=set() to bypass conditional indexing filter
+        self.stdout.write("Processing events through SafeEventsIndexer...")
+        safe_events_indexer = SafeEventsIndexer(
+            ethereum_client,
+            confirmations=0,
+            blocks_to_reindex_again=0,
+            ignored_initiators=set(),
+        )
+        safe_events_indexer.process_elements(logs)
 
-        # Create SafeContract
-        SafeContract.objects.create(address=validated_address, ethereum_tx=ethereum_tx)
-        self.stdout.write(
-            self.style.SUCCESS(
-                f"Created SafeContract for address={validated_address} "
-                f"with ethereum_tx={tx_hash}"
+        # Process decoded transactions to create SafeContract
+        self.stdout.write("Processing decoded transactions to create SafeContract...")
+        tx_processor = SafeTxProcessorProvider()
+        pending_txs = list(InternalTxDecoded.objects.pending_for_safes())
+        tx_processor.process_decoded_transactions(pending_txs)
+
+        # Verify SafeContract was created
+        if SafeContract.objects.filter(address=validated_address).exists():
+            self.stdout.write(
+                self.style.SUCCESS(
+                    f"Successfully created SafeContract for address={validated_address} "
+                    f"with ethereum_tx={tx_hash}"
+                )
+            )
+        else:
+            self.stdout.write(
+                self.style.WARNING(
+                    f"SafeContract not created for {validated_address} - "
+                    f"transaction may not be a Safe creation or address doesn't match"
+                )
             )
-        )
 
     def _handle_remove(self, addresses: list[str]) -> None:
         """Remove SafeContract entries."""
diff --git a/safe_transaction_service/history/tests/test_commands.py b/safe_transaction_service/history/tests/test_commands.py
@@ -27,7 +27,6 @@
 from ..services import IndexServiceProvider
 from ..tasks import logger as task_logger
 from .factories import (
-    EthereumTxFactory,
     MultisigConfirmationFactory,
     MultisigTransactionFactory,
     SafeContractFactory,
@@ -741,37 +740,100 @@ def test_validate_tx_integrity(self):
         self.assertNotIn("is not matching", text)
         self.assertNotIn("is not valid for multisig transaction", text)
 
-    def test_safe_contract_add(self):
-        """Test adding a SafeContract entry."""
+    @mock.patch(
+        "safe_transaction_service.history.management.commands.safe_contract.get_auto_ethereum_client"
+    )
+    @mock.patch(
+        "safe_transaction_service.history.management.commands.safe_contract.SafeEventsIndexer"
+    )
+    @mock.patch(
+        "safe_transaction_service.history.management.commands.safe_contract.SafeTxProcessorProvider"
+    )
+    def test_safe_contract_add(
+        self,
+        mock_tx_processor_provider: MagicMock,
+        mock_safe_events_indexer: MagicMock,
+        mock_get_ethereum_client: MagicMock,
+    ):
+        """Test adding a SafeContract entry processes events through the indexer."""
         command = "safe_contract"
 
-        address = Account.create().address
-        ethereum_tx = EthereumTxFactory()
-        tx_hash = ethereum_tx.tx_hash  # Already a hex string
+        safe_address = Account.create().address
+        tx_hash = "0x" + "ab" * 32
+
+        # Mock the ethereum client and its get_transaction_receipt method
+        mock_client = MagicMock()
+        mock_get_ethereum_client.return_value = mock_client
+        mock_client.get_transaction_receipt.return_value = {
+            "logs": [{"logIndex": 0, "transactionHash": tx_hash}],
+            "transactionHash": tx_hash,
+        }
+
+        # Mock indexer instance
+        mock_indexer_instance = MagicMock()
+        mock_safe_events_indexer.return_value = mock_indexer_instance
+
+        # Mock tx processor
+        mock_tx_processor = MagicMock()
+        mock_tx_processor_provider.return_value = mock_tx_processor
 
         self.assertEqual(SafeContract.objects.count(), 0)
 
-        # Add SafeContract
+        # Add SafeContract - command will process through indexer
         buf = StringIO()
-        call_command(command, "add", address, tx_hash, stdout=buf)
+        call_command(command, "add", safe_address, tx_hash, stdout=buf)
         output = buf.getvalue()
-        self.assertIn(f"Created SafeContract for address={address}", output)
-        self.assertIn(tx_hash, output)
+
+        # Verify get_transaction_receipt was called
+        mock_client.get_transaction_receipt.assert_called_once()
+
+        # Verify SafeEventsIndexer was created with ignored_initiators=set()
+        mock_safe_events_indexer.assert_called_once()
+        call_kwargs = mock_safe_events_indexer.call_args.kwargs
+        self.assertEqual(call_kwargs.get("ignored_initiators"), set())
+
+        # Verify process_elements was called on the indexer instance
+        mock_indexer_instance.process_elements.assert_called_once_with(
+            [{"logIndex": 0, "transactionHash": tx_hash}]
+        )
+
+        # Verify SafeTxProcessor.process_decoded_transactions was called
+        mock_tx_processor.process_decoded_transactions.assert_called_once()
+
+        self.assertIn("Processing events through SafeEventsIndexer", output)
+        self.assertIn("Processing decoded transactions to create SafeContract", output)
+
+    @mock.patch(
+        "safe_transaction_service.history.management.commands.safe_contract.get_auto_ethereum_client"
+    )
+    def test_safe_contract_add_already_exists(
+        self, mock_get_ethereum_client: MagicMock
+    ):
+        """Test adding a SafeContract that already exists shows warning."""
+        command = "safe_contract"
+
+        safe_contract = SafeContractFactory()
+        safe_address = safe_contract.address
+        tx_hash = "0x" + "ab" * 32
+
         self.assertEqual(SafeContract.objects.count(), 1)
-        self.assertTrue(SafeContract.objects.filter(address=address).exists())
 
-        # Add same address again (should show warning)
+        # Add same address again (should show warning without fetching)
         buf = StringIO()
-        call_command(command, "add", address, tx_hash, stdout=buf)
+        call_command(command, "add", safe_address, tx_hash, stdout=buf)
         output = buf.getvalue()
-        self.assertIn(f"SafeContract already exists for address: {address}", output)
+        self.assertIn(
+            f"SafeContract already exists for address: {safe_address}", output
+        )
         self.assertEqual(SafeContract.objects.count(), 1)
 
+        # Should not have called ethereum client
+        mock_get_ethereum_client.return_value.get_transaction_receipt.assert_not_called()
+
     def test_safe_contract_add_invalid_address(self):
         """Test adding with invalid address shows error."""
         command = "safe_contract"
-        ethereum_tx = EthereumTxFactory()
-        tx_hash = ethereum_tx.tx_hash  # Already a hex string
+        tx_hash = "0x" + "ab" * 32
 
         with self.assertRaises(CommandError) as context:
             call_command(command, "add", "invalid-address", tx_hash)
@@ -786,15 +848,46 @@ def test_safe_contract_add_invalid_tx_hash(self):
             call_command(command, "add", address, "invalid-tx-hash")
         self.assertIn("Invalid transaction hash", str(context.exception))
 
-    def test_safe_contract_add_nonexistent_tx(self):
-        """Test adding with non-existent tx hash shows error when fetching from RPC fails."""
+    @mock.patch(
+        "safe_transaction_service.history.management.commands.safe_contract.get_auto_ethereum_client"
+    )
+    def test_safe_contract_add_nonexistent_tx(
+        self, mock_get_ethereum_client: MagicMock
+    ):
+        """Test adding with non-existent tx hash shows error when receipt not found."""
         command = "safe_contract"
         address = Account.create().address
         fake_tx_hash = "0x" + "a" * 64
 
+        # Mock the ethereum client to return None for receipt
+        mock_client = MagicMock()
+        mock_get_ethereum_client.return_value = mock_client
+        mock_client.get_transaction_receipt.return_value = None
+
         with self.assertRaises(CommandError) as context:
             call_command(command, "add", address, fake_tx_hash)
-        self.assertIn("Failed to fetch transaction", str(context.exception))
+        self.assertIn("not found", str(context.exception))
+
+    @mock.patch(
+        "safe_transaction_service.history.management.commands.safe_contract.get_auto_ethereum_client"
+    )
+    def test_safe_contract_add_no_logs(self, mock_get_ethereum_client: MagicMock):
+        """Test adding with tx that has no logs shows error."""
+        command = "safe_contract"
+        address = Account.create().address
+        tx_hash = "0x" + "ab" * 32
+
+        # Mock the ethereum client to return receipt with no logs
+        mock_client = MagicMock()
+        mock_get_ethereum_client.return_value = mock_client
+        mock_client.get_transaction_receipt.return_value = {
+            "logs": [],
+            "transactionHash": tx_hash,
+        }
+
+        with self.assertRaises(CommandError) as context:
+            call_command(command, "add", address, tx_hash)
+        self.assertIn("has no logs", str(context.exception))
 
     def test_safe_contract_remove(self):
         """Test removing SafeContract entries."""
diff --git a/safe_transaction_service/history/tests/test_safe_events_indexer.py b/safe_transaction_service/history/tests/test_safe_events_indexer.py
