From f1021a7c5ca6f54ce50238fae5e0f2cb0a799034 Mon Sep 17 00:00:00 2001 From: Uxio Fuentefria <6909403+Uxio0@users.noreply.github.com> Date: Mon, 18 Nov 2024 14:22:44 +0100 Subject: [PATCH] Add a table for not valid tokens - Not valid tokens are retrieved again and again after cache expires - Add a simple list of not valid tokens so RPC call is not required - Closes #2283 --- README.md | 3 +++ .../history/services/balance_service.py | 4 +-- .../history/tests/test_balance_service.py | 10 +++---- .../history/tests/test_views_v2.py | 2 +- .../tokens/migrations/0014_tokennotvalid.py | 26 +++++++++++++++++++ safe_transaction_service/tokens/models.py | 15 +++++++++++ .../tokens/tests/test_models.py | 14 +++++----- 7 files changed, 60 insertions(+), 14 deletions(-) create mode 100644 safe_transaction_service/tokens/migrations/0014_tokennotvalid.py diff --git a/README.md b/README.md index 71b9bf73b..dc2057e7f 100644 --- a/README.md +++ b/README.md @@ -200,6 +200,9 @@ https://docs.safe.global/api-supported-networks ### 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. +### Why is my ERC20 token not indexed? +For an ERC20 token to be indexed it needs to have `name`, `symbol`, `decimals` and `balanceOf()`, otherwise the service will ignore it and add it to the `TokenNotValid` model. + ## Troubleshooting ### Issues installing grpc on an Apple silicon system diff --git a/safe_transaction_service/history/services/balance_service.py b/safe_transaction_service/history/services/balance_service.py index 3ac4a8a22..a48a41973 100644 --- a/safe_transaction_service/history/services/balance_service.py +++ b/safe_transaction_service/history/services/balance_service.py @@ -85,7 +85,7 @@ def __init__(self, ethereum_client: EthereumClient, redis: Redis): maxsize=4096, ttl=60 * 30 ) # 2 hours of caching - def _filter_addresses( + def _filter_tokens( self, erc20_addresses: Sequence[ChecksumAddress], only_trusted: bool, @@ -200,7 +200,7 @@ def _get_page_erc20_balances( for address in all_erc20_addresses: # Store tokens in database if not present self.get_token_info(address) # This is cached - erc20_addresses = self._filter_addresses( + erc20_addresses = self._filter_tokens( all_erc20_addresses, only_trusted, exclude_spam ) # Total count should take into account the request filters diff --git a/safe_transaction_service/history/tests/test_balance_service.py b/safe_transaction_service/history/tests/test_balance_service.py index 3a6cfa55b..db320e8d3 100644 --- a/safe_transaction_service/history/tests/test_balance_service.py +++ b/safe_transaction_service/history/tests/test_balance_service.py @@ -30,7 +30,7 @@ def test_get_token_info(self): self.assertEqual(token_info.symbol, token_db.symbol) self.assertEqual(token_info.decimals, token_db.decimals) - def test_filter_addresses(self): + def test_filter_tokens(self): balance_service = self.balance_service db_not_trusted_addresses = [ TokenFactory(trusted=False, spam=False).address for _ in range(3) @@ -65,12 +65,12 @@ def test_filter_addresses(self): ) self.assertCountEqual( - balance_service._filter_addresses(addresses, False, False), expected_address + balance_service._filter_tokens(addresses, False, False), expected_address ) expected_address = db_trusted_addresses self.assertCountEqual( - balance_service._filter_addresses(addresses, True, False), expected_address + balance_service._filter_tokens(addresses, True, False), expected_address ) Token.objects.filter(address=db_events_bugged_erc20_address).update( @@ -78,7 +78,7 @@ def test_filter_addresses(self): ) expected_address = db_trusted_addresses + [db_events_bugged_erc20_address] self.assertCountEqual( - balance_service._filter_addresses(addresses, True, False), expected_address + balance_service._filter_tokens(addresses, True, False), expected_address ) expected_address = ( @@ -87,5 +87,5 @@ def test_filter_addresses(self): + [db_events_bugged_erc20_address] ) self.assertCountEqual( - balance_service._filter_addresses(addresses, False, True), expected_address + balance_service._filter_tokens(addresses, False, True), expected_address ) diff --git a/safe_transaction_service/history/tests/test_views_v2.py b/safe_transaction_service/history/tests/test_views_v2.py index f7787707e..52b51ec8b 100644 --- a/safe_transaction_service/history/tests/test_views_v2.py +++ b/safe_transaction_service/history/tests/test_views_v2.py @@ -684,7 +684,7 @@ def test_safe_pagination_balances_view(self): ERC20TransferFactory(address=other_erc20.address, to=safe_address) with mock.patch( - "safe_transaction_service.history.services.balance_service.BalanceService._filter_addresses", + "safe_transaction_service.history.services.balance_service.BalanceService._filter_tokens", return_value=[erc20.address, other_erc20.address], ): response = self.client.get( diff --git a/safe_transaction_service/tokens/migrations/0014_tokennotvalid.py b/safe_transaction_service/tokens/migrations/0014_tokennotvalid.py new file mode 100644 index 000000000..c621eecd3 --- /dev/null +++ b/safe_transaction_service/tokens/migrations/0014_tokennotvalid.py @@ -0,0 +1,26 @@ +# Generated by Django 5.0.9 on 2024-11-12 13:38 + +from django.db import migrations + +import safe_eth.eth.django.models + + +class Migration(migrations.Migration): + + dependencies = [ + ("tokens", "0013_token_logo_uri"), + ] + + operations = [ + migrations.CreateModel( + name="TokenNotValid", + fields=[ + ( + "address", + safe_eth.eth.django.models.EthereumAddressBinaryField( + primary_key=True, serialize=False + ), + ), + ], + ), + ] diff --git a/safe_transaction_service/tokens/models.py b/safe_transaction_service/tokens/models.py index bf691a202..2d124d9d6 100644 --- a/safe_transaction_service/tokens/models.py +++ b/safe_transaction_service/tokens/models.py @@ -86,6 +86,10 @@ def create(self, **kwargs): def create_from_blockchain( self, token_address: ChecksumAddress ) -> Optional["Token"]: + if TokenNotValid.objects.filter(address=token_address).exists(): + # If token is not valid, ignore it + return None + ethereum_client = get_auto_ethereum_client() if token_address in ENS_CONTRACTS_WITH_TLD: # Special case for ENS return self.create( @@ -118,6 +122,7 @@ def create_from_blockchain( logger.debug( "Cannot find anything on blockchain for token=%s", token_address ) + TokenNotValid.objects.create(address=token_address) return None # Ignore tokens with empty name or symbol @@ -125,6 +130,7 @@ def create_from_blockchain( logger.warning( "Token with address=%s has not name or symbol", token_address ) + TokenNotValid.objects.create(address=token_address) return None name_and_symbol: list[str] = [] @@ -308,6 +314,15 @@ def get_price_address(self) -> ChecksumAddress: return self.copy_price or self.address +class TokenNotValid(models.Model): + """ + Stores information about tokens not valid (missing name or symbol, for example), so they are not requested + again to the RPC + """ + + address = EthereumAddressBinaryField(primary_key=True) + + class TokenListToken(TypedDict): symbol: str name: str diff --git a/safe_transaction_service/tokens/tests/test_models.py b/safe_transaction_service/tokens/tests/test_models.py index 573886140..8808af902 100644 --- a/safe_transaction_service/tokens/tests/test_models.py +++ b/safe_transaction_service/tokens/tests/test_models.py @@ -14,7 +14,7 @@ ZerionUniswapV2TokenAdapterClient, ) from ..exceptions import TokenListRetrievalException -from ..models import Token, TokenManager +from ..models import Token, TokenManager, TokenNotValid from ..models import logger as token_model_logger from .factories import TokenFactory, TokenListFactory from .mocks import token_list_mock @@ -196,11 +196,10 @@ def test_create_from_blockchain_with_string_null_data(self, get_info: MagicMock) ), ) def test_create_from_blockchain_with_empty_name(self, get_info: MagicMock): - self.assertIsNone( - Token.objects.create_from_blockchain( - "0xBB9bc244D798123fDe783fCc1C72d3Bb8C189413" - ) - ) + self.assertEqual(TokenNotValid.objects.count(), 0) + address = "0xBB9bc244D798123fDe783fCc1C72d3Bb8C189413" + self.assertIsNone(Token.objects.create_from_blockchain(address)) + self.assertEqual(TokenNotValid.objects.get(address=address).address, address) @mock.patch.object(TokenManager, "create", side_effect=ValueError) @mock.patch.object( @@ -225,6 +224,9 @@ def test_create_from_blockchain_error(self, get_info: MagicMock, create: MagicMo cm.output[0], ) + # Token should not be marked as not valid if there's a blockchain error + self.assertEqual(TokenNotValid.objects.count(), 0) + class TestTokenListModel(TestCase): @mock.patch("requests.get")