Merge branch 'safe-global:master' into master

.env.test

@@ -11,4 +11,4 @@ ETHEREUM_NODE_URL=http://localhost:8545
 ETHEREUM_TRACING_NODE_URL=http://localhost:8545
 ETH_HASH_BACKEND=pysha3
 ENABLE_ANALYTICS=True
-EVENTS_QUEUE_URL=amqp://guest:guest@rabbitmq/
+EVENTS_QUEUE_URL=amqp://guest:guest@localhost:5672/

README.md

@@ -142,11 +142,27 @@ docker exec -it safe-transaction-service-web-1 python manage.py createsuperuser
 - [v1.3.0 L2](https://github.com/safe-global/safe-deployments/blob/main/src/assets/v1.3.0/gnosis_safe_l2.json)
 - [Other related contracts and previous Safe versions](https://github.com/safe-global/safe-deployments/blob/main/src/assets)
 
-## Troubleshooting
+## Service maintenance
 
-### Issues installing grpc on a Mac M1
+Service can run into some issues when running in production:
 
-If you face issues installing the `grpc` dependency locally (required by this project) on a M1 chip, set `GRPC_PYTHON_BUILD_SYSTEM_OPENSSL=1` and `GRPC_PYTHON_BUILD_SYSTEM_ZLIB=1` and then try to install the dependency again.
+### Indexing issues
+You can tell there are indexing issues if:
+- Executed transactions are missing from the API (`all-transactions`, `multisig-transactions`, `module-transactions`... endpoints). If you use the [Safe{Wallet} Web client](https://github.com/safe-global/safe-wallet-web) you should check what is the current state of the Safe Client Gateway cache as it might have outdated data.
+- Asset transfers (ERC20/721) are missing from `all-transactions` or `transfers` endpoints.
+- You see error logs such as "Cannot remove owner" or similar inconsistent errors when `worker-indexer` is processing decoded data.
+
+There are multiple options for this. Connect to either `web` or `worker` instances. Running commands inside of `tmux` is recommended
+(installed by default):
+- `python manage.py check_index_problems`: it will try to automatically fix missing transactions.
+Tokens related transactions (ERC20/721) will not be fixed with this method. This method will take a while, as it needs to compare
+database data with blockchain data for every Safe.
+- `python manage.py reindex_master_copies --from-block-number X --addresses 0x111 0x222`: if you know the first problematic block,
+it's faster if you trigger a manual reindex. `--addresses` argument is optional, but if you know the problematic Safes providing
+them will make reindexing **way** faster, as only those Safes will be reindexed (instead of the entire collection).
+
+If you see ERC20/ERC721 transfers missing:
+- `python manage.py reindex_erc20 --from-block-number X --addresses 0x111 0x222`: same logic as with `reindex_master_copies`.
 
 ## FAQ
 ### Why `/v1/safes/{address}` endpoint shows a nonce that indicates that a transaction was executed but the transaction is not shown or marked as executed in the other endpoints?
@@ -176,5 +192,11 @@ https://docs.safe.global/learn/safe-core/safe-core-api/available-services
 ### What means banned field in SafeContract model?
 The `banned` field in the `SafeContract` model is used to prevent indexing of certain Safes that have an unsupported `MasterCopy` or unverified proxies that have issues during indexing. This field does not remove the banned Safe and indexing can be resumed once the issue has been resolved.
 
+## Troubleshooting
+
+### Issues installing grpc on a Mac M1
+
+If you face issues installing the `grpc` dependency locally (required by this project) on a M1 chip, set `GRPC_PYTHON_BUILD_SYSTEM_OPENSSL=1` and `GRPC_PYTHON_BUILD_SYSTEM_ZLIB=1` and then try to install the dependency again.
+
 ## Contributors
 [See contributors](https://github.com/safe-global/safe-transaction-service/graphs/contributors)

config/settings/base.py

@@ -415,7 +415,7 @@
     "ETH_INTERNAL_TXS_BLOCK_PROCESS_LIMIT", default=10_000
 )
 ETH_INTERNAL_TXS_BLOCKS_TO_REINDEX_AGAIN = env.int(
-    "ETH_INTERNAL_TXS_BLOCKS_TO_REINDEX_AGAIN", default=6
+    "ETH_INTERNAL_TXS_BLOCKS_TO_REINDEX_AGAIN", default=10
 )
 ETH_INTERNAL_TXS_NUMBER_TRACE_BLOCKS = env.int(
     "ETH_INTERNAL_TXS_NUMBER_TRACE_BLOCKS", default=10
@@ -442,7 +442,7 @@
     "ETH_EVENTS_BLOCK_PROCESS_LIMIT_MAX", default=0
 )  # Maximum number of blocks to process together when searching for events. 0 == no limit.
 ETH_EVENTS_BLOCKS_TO_REINDEX_AGAIN = env.int(
-    "ETH_EVENTS_BLOCKS_TO_REINDEX_AGAIN", default=10
+    "ETH_EVENTS_BLOCKS_TO_REINDEX_AGAIN", default=20
 )  # Blocks to reindex again every indexer run when service is synced. Useful for RPCs not reliable
 ETH_EVENTS_GET_LOGS_CONCURRENCY = env.int(
     "ETH_EVENTS_GET_LOGS_CONCURRENCY", default=20
@@ -454,7 +454,7 @@
     "ETH_EVENTS_UPDATED_BLOCK_BEHIND", default=24 * 60 * 60 // 15
 )  # Number of blocks to consider an address 'almost updated'.
 ETH_REORG_BLOCKS = env.int(
-    "ETH_REORG_BLOCKS", default=100 if ETH_L2_NETWORK else 10
+    "ETH_REORG_BLOCKS", default=200 if ETH_L2_NETWORK else 10
 )  # Number of blocks from the current block number needed to consider a block valid/stable
 
 # Tokens
@@ -496,10 +496,6 @@
         )
     )
 
-ALERT_OUT_OF_SYNC_EVENTS_THRESHOLD = env.float(
-    "ALERT_OUT_OF_SYNC_EVENTS_THRESHOLD", default=0.1
-)  # Percentage of Safes allowed to be out of sync without alerting. By default 10%
-
 # Events
 # ------------------------------------------------------------------------------
 EVENTS_QUEUE_URL = env("EVENTS_QUEUE_URL", default=None)

safe_transaction_service/__init__.py

@@ -1,4 +1,4 @@
-__version__ = "4.20.3"
+__version__ = "4.21.3"
 __version_info__ = tuple(
     int(num) if num.isdigit() else num
     for num in __version__.replace("-", ".", 1).split(".")

safe_transaction_service/contracts/tx_decoder.py

@@ -539,8 +539,10 @@ def get_contract_abi(
         :param address: Contract address
         :return: Dictionary of function selects with ABIFunction if found, `None` otherwise
         """
-        abis = ContractAbi.objects.filter(contracts__address=address).values_list(
-            "abi", flat=True
+        abis = (
+            ContractAbi.objects.filter(contracts__address=address)
+            .order_by("relevance")
+            .values_list("abi", flat=True)
         )
         if abis:
             return self._generate_selectors_with_abis_from_abi(abis[0])
@@ -564,7 +566,7 @@ def get_abi_function(
                     and selector in contract_selectors_with_abis
                 ):
                     # If the selector is available in the abi specific for the address we will use that one
-                    # Otherwise we fallback to the general abi that matches the selector
+                    # Otherwise we fall back to the general abi that matches the selector
                     return contract_selectors_with_abis[selector]
             return self.fn_selectors_with_abis[selector]
 

safe_transaction_service/history/indexers/erc20_events_indexer.py

@@ -27,13 +27,15 @@
 class Erc20EventsIndexerProvider:
     def __new__(cls):
         if not hasattr(cls, "instance"):
-            from django.conf import settings
-
-            cls.instance = Erc20EventsIndexer(
-                EthereumClient(settings.ETHEREUM_NODE_URL)
-            )
+            cls.instance = cls.get_new_instance()
         return cls.instance
 
+    @classmethod
+    def get_new_instance(cls) -> "Erc20EventsIndexer":
+        from django.conf import settings
+
+        return Erc20EventsIndexer(EthereumClient(settings.ETHEREUM_NODE_URL))
+
     @classmethod
     def del_singleton(cls):
         if hasattr(cls, "instance"):
@@ -227,9 +229,19 @@ def get_minimum_block_number(
     ) -> Optional[int]:
         return IndexingStatus.objects.get_erc20_721_indexing_status().block_number
 
-    def update_monitored_address(
+    def update_monitored_addresses(
         self, addresses: Sequence[str], from_block_number: int, to_block_number: int
-    ) -> int:
-        return int(
-            IndexingStatus.objects.set_erc20_721_indexing_status(to_block_number + 1)
+    ) -> bool:
+        # Keep indexing going on the next block
+        new_to_block_number = to_block_number + 1
+        updated = IndexingStatus.objects.set_erc20_721_indexing_status(
+            new_to_block_number, from_block_number=from_block_number
         )
+        if not updated:
+            logger.warning(
+                "%s: Possible reorg - Cannot update erc20_721 indexing status from-block-number=%d to-block-number=%d",
+                self.__class__.__name__,
+                from_block_number,
+                to_block_number,
+            )
+        return updated

safe_transaction_service/history/indexers/ethereum_indexer.py

@@ -256,9 +256,9 @@ def get_not_updated_addresses(
         )
         return not_updated_addresses
 
-    def update_monitored_address(
+    def update_monitored_addresses(
         self, addresses: Sequence[str], from_block_number: int, to_block_number: int
-    ) -> int:
+    ) -> bool:
         """
         :param addresses: Addresses to have the block number updated
         :param from_block_number: Make sure that no reorg has happened checking that block number was not rollbacked
@@ -278,14 +278,14 @@ def update_monitored_address(
             **{
                 "address__in": addresses,
                 self.database_field
-                + "__gte": from_block_number
-                - 1,  # Protect in case of reorg
+                + "__gte": from_block_number,  # Protect in case of reorg
                 self.database_field
                 + "__lt": new_to_block_number,  # Don't update to a lower block number
             }
         ).update(**{self.database_field: new_to_block_number})
 
-        if updated_addresses != len(addresses):
+        all_updated = updated_addresses == len(addresses)
+        if not all_updated:
             logger.warning(
                 "%s: Possible reorg - Cannot update all indexed addresses... Updated %d/%d addresses "
                 "from-block-number=%d to-block-number=%d",
@@ -404,7 +404,13 @@ def process_addresses(
 
         processed_elements = self.process_elements(elements)
 
-        self.update_monitored_address(addresses, from_block_number, to_block_number)
+        if not self.update_monitored_addresses(
+            addresses, from_block_number, to_block_number
+        ):
+            raise ValueError(
+                "Possible reorg, indexed addresses were updated while indexer was running"
+            )
+
         return processed_elements, from_block_number, to_block_number, updated
 
     def start(self) -> Tuple[int, int]:

safe_transaction_service/history/indexers/internal_tx_indexer.py

@@ -27,17 +27,21 @@
 class InternalTxIndexerProvider:
     def __new__(cls):
         if not hasattr(cls, "instance"):
-            from django.conf import settings
+            cls.instance = cls.get_new_instance()
+        return cls.instance
 
-            if settings.ETH_INTERNAL_NO_FILTER:
-                instance_class = InternalTxIndexerWithTraceBlock
-            else:
-                instance_class = InternalTxIndexer
+    @classmethod
+    def get_new_instance(cls) -> "InternalTxIndexer":
+        from django.conf import settings
 
-            cls.instance = instance_class(
-                EthereumClient(settings.ETHEREUM_TRACING_NODE_URL),
-            )
-        return cls.instance
+        if settings.ETH_INTERNAL_NO_FILTER:
+            instance_class = InternalTxIndexerWithTraceBlock
+        else:
+            instance_class = InternalTxIndexer
+
+        return instance_class(
+            EthereumClient(settings.ETHEREUM_TRACING_NODE_URL),
+        )
 
     @classmethod
     def del_singleton(cls):

safe_transaction_service/history/indexers/proxy_factory_indexer.py

@@ -21,14 +21,16 @@
 class ProxyFactoryIndexerProvider:
     def __new__(cls):
         if not hasattr(cls, "instance"):
-            from django.conf import settings
-
-            cls.instance = ProxyFactoryIndexer(
-                EthereumClient(settings.ETHEREUM_NODE_URL)
-            )
+            cls.instance = cls.get_new_instance()
 
         return cls.instance
 
+    @classmethod
+    def get_new_instance(cls) -> "ProxyFactoryIndexer":
+        from django.conf import settings
+
+        return ProxyFactoryIndexer(EthereumClient(settings.ETHEREUM_NODE_URL))
+
     @classmethod
     def del_singleton(cls):
         if hasattr(cls, "instance"):

safe_transaction_service/history/indexers/safe_events_indexer.py

@@ -31,11 +31,15 @@
 class SafeEventsIndexerProvider:
     def __new__(cls):
         if not hasattr(cls, "instance"):
-            from django.conf import settings
-
-            cls.instance = SafeEventsIndexer(EthereumClient(settings.ETHEREUM_NODE_URL))
+            cls.instance = cls.get_new_instance()
         return cls.instance
 
+    @classmethod
+    def get_new_instance(cls) -> "SafeEventsIndexer":
+        from django.conf import settings
+
+        return SafeEventsIndexer(EthereumClient(settings.ETHEREUM_NODE_URL))
+
     @classmethod
     def del_singleton(cls):
         if hasattr(cls, "instance"):

safe_transaction_service/history/migrations/0073_safe_apps_links.py

@@ -0,0 +1,32 @@
+"""
+Migrate Safe Apps links from apps.gnosis-safe.io -> apps-portal.safe.global
+
+Generated by Django 4.2.1 on 2023-06-26 14:18
+"""
+
+from django.db import migrations
+
+# No way to do this efficiently using Django's ORM, so using a raw SQL
+migrate_safe_apps_links_sql = """
+    UPDATE history_multisigtransaction
+    SET origin = replace(origin::text, 'https://apps.gnosis-safe.io', 'https://apps-portal.safe.global')::jsonb
+    WHERE origin->>'url' LIKE 'https://apps.gnosis-safe.io%'
+"""
+
+reverse_migrate_safe_apps_links_sql = """
+    UPDATE history_multisigtransaction
+    SET origin = replace(origin::text, 'https://apps-portal.safe.global', 'https://apps.gnosis-safe.io')::jsonb
+    WHERE origin->>'url' LIKE 'https://apps-portal.safe.global%'
+"""
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("history", "0072_safecontract_banned_and_more"),
+    ]
+
+    operations = [
+        migrations.RunSQL(
+            migrate_safe_apps_links_sql, reverse_sql=reverse_migrate_safe_apps_links_sql
+        )
+    ]

safe_transaction_service/history/models.py

@@ -166,12 +166,20 @@ class IndexingStatusManager(models.Manager):
     def get_erc20_721_indexing_status(self) -> "IndexingStatus":
         return self.get(indexing_type=IndexingStatusType.ERC20_721_EVENTS.value)
 
-    def set_erc20_721_indexing_status(self, block_number: int) -> bool:
-        return bool(
-            self.filter(indexing_type=IndexingStatusType.ERC20_721_EVENTS.value).update(
-                block_number=block_number
-            )
-        )
+    def set_erc20_721_indexing_status(
+        self, block_number: int, from_block_number: Optional[int] = None
+    ) -> bool:
+        """
+
+        :param block_number:
+        :param from_block_number: If provided, only update the field if bigger than `from_block_number`, to protect
+                                  from reorgs
+        :return:
+        """
+        queryset = self.filter(indexing_type=IndexingStatusType.ERC20_721_EVENTS.value)
+        if from_block_number is not None:
+            queryset = queryset.filter(block_number__gte=from_block_number)
+        return bool(queryset.update(block_number=block_number))
 
 
 class IndexingStatus(models.Model):