Skip to content

Commit

Permalink
Add a table for not valid tokens
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
Uxio0 committed Nov 18, 2024
1 parent 3635d19 commit b68639d
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 14 deletions.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions safe_transaction_service/history/services/balance_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions safe_transaction_service/history/tests/test_balance_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -65,20 +65,20 @@ 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(
trusted=True
)
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 = (
Expand All @@ -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
)
2 changes: 1 addition & 1 deletion safe_transaction_service/history/tests/test_views_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
26 changes: 26 additions & 0 deletions safe_transaction_service/tokens/migrations/0014_tokennotvalid.py
Original file line number Diff line number Diff line change
@@ -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
),
),
],
),
]
15 changes: 15 additions & 0 deletions safe_transaction_service/tokens/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -118,13 +122,15 @@ 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
if not erc_info.name or not erc_info.symbol:
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] = []
Expand Down Expand Up @@ -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
Expand Down
14 changes: 8 additions & 6 deletions safe_transaction_service/tokens/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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")
Expand Down

0 comments on commit b68639d

Please sign in to comment.